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

Don't set s3api create-bucket LocationConstraint configuration option for us-east-1 region #19248

Merged

Conversation

charlesjmorgan
Copy link
Member

Description

Make the LocationConstraint create-bucket-configuration property optional depending on which aws region is used for ci. LocationConstraint can't be set if the AWS region used is us-east-1. This doesn't affect Trino, but could affect forks

Additional context and related issues

Release notes

(X) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text:

# Section
* Fix some things. ({issue}`issuenumber`)

@cla-bot cla-bot bot added the cla-signed label Oct 3, 2023
@charlesjmorgan charlesjmorgan force-pushed the cm/optional-region-config-s3-ci branch from 364a510 to d3cb0f4 Compare October 3, 2023 21:41
@charlesjmorgan charlesjmorgan changed the title Fix special us-east-1 treatment by aws s3api create-bucket Don't set s3api create-bucket LocationConstraint configuration option for us-east-1 region Oct 3, 2023
@charlesjmorgan
Copy link
Member Author

Need to run ci with secrets, not sure if that can be done in this branch or if a clone of this branch should be made in the Trino repo

@charlesjmorgan charlesjmorgan force-pushed the cm/optional-region-config-s3-ci branch from d3cb0f4 to ae9c700 Compare October 3, 2023 21:44
@electrum
Copy link
Member

electrum commented Oct 4, 2023

Update to #19060

@electrum
Copy link
Member

electrum commented Oct 4, 2023

/test-with-secrets sha=ae9c7007801f1e350c59125b6cc977af31f7693f


# LocationConstraint configuration property is not allowed for us-east-1 AWS region
if [ "${AWS_REGION}" == 'us-east-1' ]; then
optBucketConfiguration=("")
Copy link
Member

Choose a reason for hiding this comment

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

This should be an empty array, not an array with an empty string

Copy link
Member Author

Choose a reason for hiding this comment

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

updated

@github-actions
Copy link

github-actions bot commented Oct 4, 2023

The CI workflow run with tests that require additional secrets finished as failure: https://github.com/trinodb/trino/actions/runs/6401035079

Copy link
Member

@bitweld bitweld left a comment

Choose a reason for hiding this comment

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

LGTM % Jan comment

Make the LocationConstraint create-bucket-configuration property optional
depending on which aws region is used to run ci. LocationConstraint
can't be set if the AWS_REGION used is us-east-1
@charlesjmorgan charlesjmorgan force-pushed the cm/optional-region-config-s3-ci branch from ae9c700 to d056382 Compare October 5, 2023 02:41
@charlesjmorgan
Copy link
Member Author

Updated. PTAL @electrum

Copy link
Member

@bitweld bitweld left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants