fix: add DB-Name header to get import progress REST API requests#3415
fix: add DB-Name header to get import progress REST API requests#3415xiaowshi wants to merge 3 commits intomilvus-io:masterfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: xiaowshi The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Signed-off-by: Xiaowei Shi <shallwe.shih@gmail.com>
0126dec to
0f84f4b
Compare
|
/lgtm |
pymilvus-bot
left a comment
There was a problem hiding this comment.
The fix correctly adds db_name parameter to get_import_progress and injects it as the DB-Name HTTP header via _http_headers. However, the issue title and body explicitly call out both get_import_progress and list_import_jobs as affected.
Looking at list_import_jobs in master, it already accepts a db_name parameter but passes it only as a JSON body field ("dbName": db_name), not as the DB-Name HTTP header. Since the RBAC PrivilegeInterceptor reads the database name from the HTTP header, list_import_jobs has the same 403 problem for non-default databases with RBAC enabled.
Please also pass db_name to _post_request (and hence _http_headers) in list_import_jobs, similar to the fix already applied to get_import_progress.
Example (in list_import_jobs):
resp = _post_request(
url=request_url, api_key=api_key, params=params, verify=verify, cert=cert,
db_name=db_name, # add this
**kwargs,
)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3415 +/- ##
==========================================
+ Coverage 90.79% 90.97% +0.18%
==========================================
Files 64 64
Lines 13945 13982 +37
==========================================
+ Hits 12661 12720 +59
+ Misses 1284 1262 -22 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@xiaowshi, please add ut to increase the patch cov into 90% so that the patch checks will pass. |
|
New changes are detected. LGTM label has been removed. |
fba585e to
3d57e5e
Compare
|
Please run |
Signed-off-by: Xiaowei Shi <shallwe.shih@gmail.com>
2ddbad8 to
24c17e4
Compare
issue #3413