Feat/drs 1.5#441
Conversation
|
The style in this PR agrees with This formatting comment was generated automatically by a script in uc-cdis/wool. |
Please find the detailed integration test report here Please find the Github Action logs here |
c21469d to
af41355
Compare
|
Test summary after running integration tests
Test summary after rerunning failed integration tests
Please find the detailed integration test report here Please find the detailed integration test report after rerunning failed tests here Please find the Github Action logs here |
|
Test summary after running integration tests
Test summary after rerunning failed integration tests
Please find the detailed integration test report here Please find the detailed integration test report after rerunning failed tests here Please find the Github Action logs here |
|
Test summary after running integration tests
Test summary after rerunning failed integration tests
Please find the detailed integration test report here Please find the detailed integration test report after rerunning failed tests here Please find the Github Action logs here |
|
Test summary after running integration tests
Test summary after rerunning failed integration tests
Please find the detailed integration test report here Please find the detailed integration test report after rerunning failed tests here Please find the Github Action logs here |
|
Test summary after running integration tests
Test summary after rerunning failed integration tests
Please find the detailed integration test report here Please find the detailed integration test report after rerunning failed tests here Please find the Github Action logs here |
|
Test summary after running integration tests
Test summary after rerunning failed integration tests
Please find the detailed integration test report here Please find the detailed integration test report after rerunning failed tests here Please find the Github Action logs here |
|
Test summary after running integration tests
Test summary after rerunning failed integration tests
Please find the detailed integration test report here Please find the detailed integration test report after rerunning failed tests here Please find the Github Action logs here |
|
Test summary after running integration tests
Test summary after rerunning failed integration tests
Please find the detailed integration test report here Please find the detailed integration test report after rerunning failed tests here Please find the Github Action logs here |
| location_type = location.split(":")[ | ||
| 0 | ||
| ] # (s3, gs, ftp, gsiftp, globus, htsget, https, file) | ||
| cloud = blueprint.cloud_provider_map.get(location_type) |
There was a problem hiding this comment.
Per the feature doc we need to support http mappings e.g.
CLOUD_PROVIDER_MAP = {
"s3": "aws",
"gs": "gcp",
"az": "azure",
"https": {
"m3.aicommons.com/ai": "aws",
"storage.googleapis.com": "gcp",
},
}
For an HTTPS URL, location_type is "https" and if CLOUD_PROVIDER_MAP contains a nested dict under "https", cloud becomes that nested dict object. We probably need a helper function here and in the non drs indexd called like derive_cloud(url) that handles both simple scheme mappings and nested HTTPS prefix mappings
There was a problem hiding this comment.
Do we have cases for this in gen3-code-vigil?
There was a problem hiding this comment.
no it doesn't. And tbh it may be out of scope in the very short term
There was a problem hiding this comment.
Let me add the cases here. We can backlog a ticket on GQE.
There was a problem hiding this comment.
http mappings added.
|
are we adding the bulk drs object endpoint in this PR? |
Yes - this fell through the tracks on tasking 🤦 I'll push an update later today |
Integration TestsTest summary after running integration tests
Test summary after rerunning failed integration tests
Please find the detailed integration test report here Please find the detailed integration test report after rerunning failed tests here Please find the Github Action logs here |
Integration TestsFailed to Prepare CI environment. The following pods are not in healthy state: Please find the Github Action logs here |
Integration TestsFailed to Prepare CI environment Please find the Github Action logs here |
Integration TestsTest summary after running integration tests
Test summary after rerunning failed integration tests
Please find the detailed integration test report here Please find the detailed integration test report after rerunning failed tests here Please find the Github Action logs here |
Integration TestsFailed to Prepare CI environment. The following pods are not in healthy state: Please find the Github Action logs here |
Integration TestsTest summary after running integration tests
Test summary after rerunning failed integration tests
Please find the detailed integration test report here Please find the detailed integration test report after rerunning failed tests here Please find the Github Action logs here |
Integration TestsTest summary after running integration tests
Test summary after rerunning failed integration tests
Please find the detailed integration test report here Please find the detailed integration test report after rerunning failed tests here Please find the Github Action logs here |
Integration TestsTest summary after running integration tests
Test summary after rerunning failed integration tests
Please find the detailed integration test report here Please find the detailed integration test report after rerunning failed tests here Please find the Github Action logs here |
Integration TestsTest summary after running integration tests
Test summary after rerunning failed integration tests
Please find the detailed integration test report here Please find the detailed integration test report after rerunning failed tests here Please find the Github Action logs here |
New endpoint: OPTIONS /objects/{object_id} - returns a list of Authorizations
that can be used to determine how to authorize requests to get object or post
object.
New endpoint: OPTIONS /objects - Returns authorization information for
multiple DrsObjects at once.
New endpoint: POST /objects - Get info about
multiple DrsObjects
New endpoint: POST /objects/access (Bulk Access URLs) -
Returns an array of URL objects that can be used to download of multiple
DrsObjects.
Update to DrsObject's access_methods: region, cloud, available and
authorization are added to fields.
CLOUD_PROVIDER_MAP and DRS_AUTHORIZATION_METADATA environment variables should
be provided to support the cloud field for access_methods and the
authorizations field for access_methods. See docs for more information.
port changes for single option
port bulk changes
add bulk abstract
pytest fixes
test
test2
test whether open PR re-runs with a change
Work in progess: PR feedback updates
add more bulk auth option tests
fix flaky tests
update docstring
test commit
revert change
code changes for cloud field
fix broken tests
add test for cloud field
Initial changes for region field
remove debugging print statement from test
more changes for region field and available field
fix for unit tests
fix test_update_urls_metadata
more test fixes
switch to string 'true'
more test fixes
fix test_cloud_field
swap fullmatch for search
remove regexp vulnerability and add test for region field
Fix new unit tests
use get_doc instead of manual creation
remove extra imports from bottom
try this fix
clear cache too
local unit test fixes
Update settings files to try pulling in environment variables first
drs 1.5 service info support
Run drs 1.5 integration tests
Fix access_method overrides
Rename access_method test
More available fixes
drs 1.5 helm branch for integration_tests
Add cloud provider domain linking
Add get_drs_objects endpoint WIP
Work in progress - saving updates
WIP - saving updates
Work in progress - refactored single resolver/tests
Clean up bulk tests/helpers
pytest fix
Updates for PR review ( passport handling + POST)
auth nesting updated within access_methods
add single post, make names more consistent
improve readability
Fix logging
Fix merge issues and update test_drs authorization tests
Fix Exceptions in DRS to use IndexdUnexpectedError
Safely access default_passport_issuer
Add default issuers to authz, fix cloud drs unit tests
Fix supported_types for open access
Debugging logs
Move logger init to after blueprint registration
Fix defaults for authz, removing debug logs
Fix test_single_path_not_found test to use defaults
Co-authored-by: jacob50231 <jacobdaugherty@uchicago.edu>
Co-authored-by: nrandorf <randorf@uchicago.edu>
Co-authored-by: SpencerAxelrod <saxelrod@uchicago.edu>
Integration TestsFailed to Prepare CI environment. The following pods are not in healthy state: Please find the Github Action logs here |
| def get_doc( | ||
| has_version=True, | ||
| urls=list(), | ||
| urls=None, |
|
|
||
| assert res.status_code == 200 | ||
| assert res.json == expected_info | ||
| data = res.json |
There was a problem hiding this comment.
I would have kept the dictionary equality assertion the same and just changed expected_info dictionary. Truly a nothing burger though. Same test excluding the maxBulkRequestLength being a < instead of ==
MichaelLukowski
left a comment
There was a problem hiding this comment.
Looks good!
One small nitpick but it should be fine
| if "urls" in record and record["urls"]: | ||
| for url in record["urls"]: | ||
| if url.startswith("s3://") and url not in region: | ||
| bucket_name = url.split("/")[2] |
There was a problem hiding this comment.
should we urlparse for this instead of split and index? Just in case a s3 url is malformed.
There was a problem hiding this comment.
Good point about malformed handling. urlparse won't provide validation, but we can add in some conditional handling for malformed scenario (i.e. if the scheme is unexpected, we can skip the split and index entirely).
There was a problem hiding this comment.
On further review, I think we can leave this as is because the existing code already avoids appending url data if the input is malformed. A match in the subsequent lookup_bucket_region() helper is required and if the match is not found, then region information for the malformed url defaults to an empty string.
Link to JIRA ticket if there is one:
New Features
OPTIONS /objects/{object_id}- returns a list of Authorizations that can be used to determine how to authorize requests to get object or post object.OPTIONS /objects- Returns authorization information for multiple DrsObjects at once.POST /objects- Get info about multiple DrsObjectsPOST /objects/access (Bulk Access URLs)- Returns an array of URL objects that can be used to download of multiple DrsObjects.access_methods:region,cloud,availableandauthorizationare added to fields.Breaking Changes
Bug Fixes
Improvements
Dependency updates
Deployment changes