Bug/fix credentials s3 buildcache update#31391
Conversation
josephsnyder
force-pushed
the
bug/fix_credentials_s3_buildcache_update
branch
from
7a474c1 to
faaba06
Compare
| # Ensure most specific URLs (longest) are presented first | ||
| mirror_url_keys = mirror_dict.keys() | ||
| mirror_url_keys = sorted(mirror_url_keys, key=len, reverse=True) |
There was a problem hiding this comment.
Is it necessarily the case that the longer URL will be more "specific"? I guess I'm wondering what exactly is being done in this sort and why?
There was a problem hiding this comment.
I think that it is. Assume that the file we're looking at is s3:://test/stub/buildcache/packages.json. If we have two mirrors set up on this s3 bucket , say s3://test/stub and s3://test. The shorter URL will match "startswith" for this file, and may provide wrong connection information, even though there is a "more correct" answer.
There was a problem hiding this comment.
Ah, yes I see! It's the extra startswith() below that completes the logic.
On that note, am I reading the next few lines correctly? It looks like maybe you meant to break out early in the for loop on the first matching url? As currently written, wouldn't you fall through to the last matching entry (or the least specific, given the sort above?) 👇
|
I am in favor of merging in spite of the |
Ensure that the s3 connection made when attempting to update the content of a buildcache attempts to use the extra connection information from the mirror creation.
Fix copy/paste error for endpoint URL help which was the same as the access token
Due to the fact that nested bucket URLs would never match the string used for checking that the mirror is the same, switch the check used. Sort all mirror URLs by length to have the most specific cases first and see if the desired URL "starts with" the mirror URL.
Add execptions for long lines and fix other style errors
Use the format command to rebuild the url instead of crafing a formatted string out of known values
When a valid mirror is found, break from the loop
josephsnyder
force-pushed
the
bug/fix_credentials_s3_buildcache_update
branch
from
0c8bbeb to
a5a6c16
Compare
* Add connection information to buildcache update command Ensure that the s3 connection made when attempting to update the content of a buildcache attempts to use the extra connection information from the mirror creation. * Add unique help for endpoint URL argument Fix copy/paste error for endpoint URL help which was the same as the access token * Re-work URL checking for S3 mirrors Due to the fact that nested bucket URLs would never match the string used for checking that the mirror is the same, switch the check used. Sort all mirror URLs by length to have the most specific cases first and see if the desired URL "starts with" the mirror URL. * Long line style fixes Add execptions for long lines and fix other style errors * Use format() function to rebuild URL Use the format command to rebuild the url instead of crafing a formatted string out of known values * Add early exit for URL checking When a valid mirror is found, break from the loop
2 participants
A set of fixes for the S3 connection information found during the usage of a mirror and the buildcache command. See https://spackpm.slack.com/archives/C01UZDL35C4/p1655420904901919 for initial issue and further discussion.