Skip to content

feat: support param in sm batch #6229

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Feb 25, 2025
Merged

feat: support param in sm batch #6229

merged 11 commits into from
Feb 25, 2025

Conversation

zac-li
Copy link
Member

@zac-li zac-li commented Feb 13, 2025

Goals:

Currently, the FastAPI provided by jina can’t handle SageMaker Batch Transform requests when both the doc list and parameters (e.g., late_trunking set to True/False) need to be considered. In the CSV, there’s no way to specify parameters.

This PR introduces a way for users to set both the document list and parameters. While it’s not the cleanest solution, it works.

Essentially, the first row will be used for parameters, and it must include params_row as a signature to identify it as the parameters row. The remaining rows will still be used as doc rows.

Example:

0,params_row,retrieval.query,false,2
1,abcd
2,efgh
3,ijkl
4,mnop
5,qrst
6,uvwx
7,yzab
8,cdef
9,ghij
10,klmn

@github-actions github-actions bot added size/M area/core This issue/PR affects the core codebase area/testing This issue/PR affects testing labels Feb 13, 2025
Copy link

codecov bot commented Feb 13, 2025

Codecov Report

Attention: Patch coverage is 0% with 54 lines in your changes missing coverage. Please review.

Project coverage is 60.77%. Comparing base (c933e49) to head (3efc7f1).
Report is 39 commits behind head on master.

Files with missing lines Patch % Lines
jina/serve/runtimes/worker/http_csp_app.py 0.00% 54 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master    #6229       +/-   ##
===========================================
+ Coverage   19.02%   60.77%   +41.75%     
===========================================
  Files         150      152        +2     
  Lines       14226    14301       +75     
===========================================
+ Hits         2706     8691     +5985     
+ Misses      11520     5610     -5910     
Flag Coverage Δ
jina 60.77% <0.00%> (+41.75%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@JoanFM JoanFM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please define what is being done with more detail.

@github-actions github-actions bot added area/cicd This issue/PR affects the cicd pipeline area/housekeeping This issue/PR is housekeeping labels Feb 20, 2025
@JoanFM JoanFM force-pushed the fix-sm-param-batch branch from 9c251a4 to eac3089 Compare February 20, 2025 09:00
JoanFM
JoanFM previously approved these changes Feb 20, 2025
@JoanFM JoanFM merged commit fa7f11b into master Feb 25, 2025
123 of 128 checks passed
@JoanFM JoanFM deleted the fix-sm-param-batch branch February 25, 2025 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cicd This issue/PR affects the cicd pipeline area/core This issue/PR affects the core codebase area/housekeeping This issue/PR is housekeeping area/testing This issue/PR affects testing size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants