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.

Sign up for GitHub

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

Jump to bottom

Only run exception thrown on existing bucket for Minio #82

Merged
merged 1 commit into from
Apr 19, 2023
Merged

Only run exception thrown on existing bucket for Minio #82

merged 1 commit into from
Apr 19, 2023

Conversation

mdedetrich
Copy link
Contributor

Comment in PR has explanation

He-Pin
He-Pin approved these changes Apr 19, 2023
View reviewed changes
Copy link
Member

@He-Pin He-Pin left a comment

Choose a reason for hiding this comment

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

LGTM

@mdedetrich
Copy link
Contributor Author

@He-Pin I have a slight improvement coming up, don't merge yet

@mdedetrich mdedetrich force-pushed the only-run-exception-on-existing-bucket-for-minio branch from 4aea03a to 1e9d2e4 Compare April 19, 2023 09:03
@mdedetrich
Copy link
Contributor Author

mdedetrich commented Apr 19, 2023 •
edited

Okay I just improved the PR by also adding a test that confirms the AWS S3 behaviour. Tested it against S3 account and I can confirm that it works.

@mdedetrich mdedetrich merged commit 10b0c95 into apache:main Apr 19, 2023
48 of 50 checks passed
@mdedetrich mdedetrich deleted the only-run-exception-on-existing-bucket-for-minio branch April 19, 2023 09:16
@He-Pin
Copy link
Member

He-Pin commented Apr 19, 2023

Seems the connector part is not easy to tests😁😁😁

@mdedetrich
Copy link
Contributor Author

@He-Pin

You can read https://issues.apache.org/jira/browse/INFRA-24353?focusedCommentId=17713815&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17713815 for context but the current problem we are dealing with right now is that because these tests never ran against a real AWS account in CI a lot of tests have bit rotted (i.e. gradually broken) over time and I am now trying to fix them.

Even worse in some cases there are actual regressions and/or S3 changed their API behaviour, i.e. #81

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

@He-Pin He-Pin He-Pin approved these changes

@jrudolph jrudolph Awaiting requested review from jrudolph

@gmethvin gmethvin Awaiting requested review from gmethvin

@nvollmar nvollmar Awaiting requested review from nvollmar

@seglo seglo Awaiting requested review from seglo

@pjfanning pjfanning Awaiting requested review from pjfanning

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@mdedetrich @He-Pin