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::URI #1523

Closed
wants to merge 2 commits into from
Closed

S3::URI #1523

wants to merge 2 commits into from

Conversation

dvirtz
Copy link

@dvirtz dvirtz commented Nov 25, 2020

Issue #1507

Description of changes:

  • refactor Http::URI to a generic Net::URI.
  • add S3::S3URI class.
  • both Http::URI and S3::S3URI derive from Net::URI.

Check all that applies:

  • Did a review by yourself.
  • Added proper tests to cover this PR. (If tests are not applicable, explain.)
  • Checked if this PR is a breaking (APIs have been changed) change.
  • Checked if this PR will not introduce cross-platform inconsistent behavior.
  • Checked if this PR would require a ReadMe/Wiki update.

Check which platforms you have built SDK on to verify the correctness of this PR.

  • Linux
  • Windows
  • Android
  • MacOS
  • IOS
  • Other Platforms

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@dvirtz dvirtz marked this pull request as draft November 25, 2020 21:51
@dvirtz dvirtz mentioned this pull request Nov 25, 2020
@dvirtz dvirtz marked this pull request as ready for review November 30, 2020 13:28
@dvirtz
Copy link
Author

dvirtz commented Nov 30, 2020

I'll be happy to get some hints about if and how to update documentation.

@KaibaLopez
Copy link
Contributor

Hi @dvirtz ,
Can you provide some background on why this change is necessary? Like, what problem are you having that this fixes?

@dvirtz
Copy link
Author

dvirtz commented Nov 30, 2020

The background is shortly described in the attached issue, but to reiterate I'm getting an S3 URI from my library's clients and in order to call the S3 API I need to parse that URI into bucket, key etc. The added S3URI class solves that.

@github-actions
Copy link

Greetings! Sorry to say but this is a very old issue that is probably not getting as much attention as it deservers. We encourage you to check if this is still an issue in the latest release and if you find that this is still a problem, please feel free to open a new one.

@github-actions github-actions bot added the closing-soon This issue will automatically close in 4 days unless further comments are made. label Apr 20, 2022
@jmklix jmklix added feature-request A feature should be added or improved. needs-review This issue or pull request needs review from a core team member. and removed closing-soon This issue will automatically close in 4 days unless further comments are made. labels Apr 22, 2022
@jmklix
Copy link
Member

jmklix commented Aug 30, 2022

Closing this because this is change is not backwards compatible

@jmklix jmklix closed this Aug 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved. needs-review This issue or pull request needs review from a core team member.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants