Skip to content
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

Shorter timeout for get bucket location #6295

Merged
merged 1 commit into from
Aug 1, 2023

Conversation

johnnyaug
Copy link
Contributor

@johnnyaug johnnyaug commented Jul 31, 2023

To avoid long wait when GetBucketLocation is forbidden for the execution role.

@github-actions
Copy link

No linked issues found. Please add the corresponding issues in the pull request description.
Use GitHub automation to close the issue when a PR is merged

@johnnyaug johnnyaug added exclude-changelog PR description should not be included in next release changelog minor-change Used for PRs that don't require issue attached labels Jul 31, 2023
// calling `client.getBucketLocation(bucket)`.
if (region.equals(Region.US_Standard)) "us-east-1"
else region.toString
Region.fromValue(bucketRegion).toAWSRegion().getName()
Copy link
Contributor

Choose a reason for hiding this comment

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

What if region returns US Standard?
Will it still work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, in this case:

  1. Region.fromValue will return com.amazonaws.services.s3.model.Region.US_Standard.
  2. Then, calling toAWSRegion will return RegionUtils.getRegion("us-east-1").

Copy link
Contributor

@Jonathan-Rosenberg Jonathan-Rosenberg Aug 1, 2023

Choose a reason for hiding this comment

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

but then getName will return US again, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No: from the docs of com.amazonaws.regions.Region:

The unique system ID for this region; ex: "us-east-1".

(also tested manually)

Copy link
Contributor

@Jonathan-Rosenberg Jonathan-Rosenberg left a comment

Choose a reason for hiding this comment

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

💣

@johnnyaug johnnyaug merged commit 80662e9 into master Aug 1, 2023
39 of 43 checks passed
@johnnyaug johnnyaug deleted the metaclient/shorter_timeout_for_get_location branch August 1, 2023 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exclude-changelog PR description should not be included in next release changelog minor-change Used for PRs that don't require issue attached
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants