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

S3 eu south #1950

Merged
merged 4 commits into from
Sep 13, 2023
Merged

S3 eu south #1950

merged 4 commits into from
Sep 13, 2023

Conversation

H3199
Copy link
Contributor

@H3199 H3199 commented Sep 1, 2023

Added EU-South-1 (Milan) AWS region to s3 storage driver.

Description

EU-South was added similarly as EU-North.
I couldn't connect to EU-South region with K8ssandra's Medusa, which uses this storage driver to establish connection to AWS. After adding the region Medusa's connection worked.

Status

  • done, ready for review

Checklist (tick everything that applies)

  • Code linting (required, can be done after the PR checks)
  • Documentation
  • Tests
  • ICLA (required for bigger changes)

@@ -1458,6 +1460,14 @@ class S3EUNorth1StorageDriver(S3StorageDriver):
ex_location_name = "eu-north-1"
region_name = "eu-north-1"

class S3EUSouth1Connection(S3SignatureV4Connection):
Copy link
Member

@Kami Kami Sep 13, 2023

Choose a reason for hiding this comment

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

There is no need anymore to define a class per region. A long time ago we moved away from a class per region to region constructor argument approach. Other classes were just left in places for backward compatibility reasons.

Just adding an entry to REGION_TO_HOST_MAP dictionary is sufficient.

Related comment on a similar PR - #1821 (comment).

if region and region not in REGION_TO_HOST_MAP.keys():

asfgit pushed a commit that referenced this pull request Sep 13, 2023
@asfgit asfgit merged commit be7a993 into apache:trunk Sep 13, 2023
17 of 18 checks passed
@Kami
Copy link
Member

Kami commented Sep 13, 2023

I've removed unnecessary class for the connection and storage driver and merged change into trunk.

Thanks.

@Kami Kami added this to the v3.9.0 milestone Sep 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants