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

Fix locate endpoint bucket name test #802

Merged
merged 2 commits into from
Mar 19, 2024
Merged

Conversation

reweeden
Copy link
Contributor

#777 introduced a change to how the locate endpoint works with respect to the BucketnamePrefix parameter. Previously the bucket parameter expected the name of the bucket WITHOUT the prefix. This is arguably not the desired behavior as it requires the caller of the application to know which part of the bucket name is the prefix and to be able to strip it out in the request. However, this does create a breaking change in the API so we have a few options:

  1. Bump the version number to 2.x to reflect the fact that this is a breaking change.
  2. Decide that the old behavior was a bug and therefore we only bump the bugfix number 1.3.x. This is probably not the best option because it's possible that there are some TEA deployments out there that might be affected by this.
  3. Support both for a while before dropping support for the old behavior. This still requires doing option 1 at some point in the future but just kicks that to a later date. I don't think there's realistically any benefit to doing that over just bumping the version number now.

I think option 1 is the way to go for now. Even though this is a breaking change, AFAIK CIRRUS/cumulus doesn't use the BucketnamePrefix parameter, so those deployments won't be affected and Sentinel doesn't use the locate endpoint so those deployments shouldn't be affected either. If there are some affected deployments out there they'll have the warning of the version number bump and they can ask for support in upgrading.

Copy link
Contributor

@mattp0 mattp0 left a comment

Choose a reason for hiding this comment

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

Option 1 seems like the best for a breaking change. We could continue to support 1.3.x and bump the version here to 2.x?

@reweeden reweeden force-pushed the rew/locate-bucket-name-prefix branch from 4cca69b to ff01366 Compare March 19, 2024 19:03
@reweeden reweeden force-pushed the rew/locate-bucket-name-prefix branch from 152b7ad to 4092e31 Compare March 19, 2024 20:52
@reweeden reweeden merged commit 81c27a1 into devel Mar 19, 2024
14 of 15 checks passed
@reweeden reweeden deleted the rew/locate-bucket-name-prefix branch March 19, 2024 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants