scylla-manager: make --scylla-manager-package option more robust
#464
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ShlomiBalalis
approved these changes
Jun 8, 2023
tchaikov
reviewed
Jun 12, 2023
tchaikov
reviewed
Jun 12, 2023
tchaikov
reviewed
Jun 12, 2023
tchaikov
reviewed
Jun 12, 2023
fruch
force-pushed
the
fix_463
branch
2 times, most recently
from
234bca8
to
da421c0
Compare
tchaikov
reviewed
Jun 28, 2023
| @@ -143,10 +144,15 @@ def download_version_from_s3(url: str, target_path: str, verbose=False): | |||
| try: | |||
| metadata = s3_client.head_object(Bucket=bucket_name, Key=download_path) | |||
| except botocore.client.ClientError as ex: | |||
| if 'Not Found' in str(ex): | |||
| error_message = ex.response['Error']['Message'] | |||
There was a problem hiding this comment.
could you please revisit my previous comment, and review the document. for a sample of how the error is examined, see https://github.com/aws/aws-cli/blob/855cd0ddb3c7fd47cab02a390281f29cc9bc744c/awscli/customizations/s3uploader.py#L123-L127
i think the "Message" field is supposed to be consumed by human being, not to be checked by an application.
There was a problem hiding this comment.
the code was 404, which is Not Found
and 403 which is Forbidden.
from my POV it the same, and less clear or readable
* now it would first try to download from s3 via boto api (faster) * local tar.gz file of manager can be used (no need to have it on s3/http) * hash to identify the specific version now based on ETag and not on the url path itself. Fixes: scylladb#463
on case of old artifacts which aren't configured to be publicly available on s3 (Forbidden), we should fallback to download via the http proxy (downloads.scylladb.com)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes: #463
Testing