Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

dropped AWS signature #56

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

mreinsch
Copy link
Contributor

Copy link
Owner

@ericcj ericcj left a comment

Choose a reason for hiding this comment

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

What a wonderful simplification. Were you able to test that it works?

@@ -3,7 +3,6 @@
module AmzSpApi
class SpConfiguration < Configuration
attr_accessor :refresh_token, :client_id, :client_secret, :sandbox, :region,
:aws_access_key_id, :aws_secret_access_key, :credentials_provider, # either access key or credentials_provider for AWS Signer, e.g. Aws::STS::Client
Copy link
Owner

Choose a reason for hiding this comment

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

these incompatible API changes would force this to be part of a major version change whereas if we left them in place and even better added this we could roll it out in a patch

  extend Gem::Deprecate
  deprecate :aws_access_key_id=, :none, 2025, 1
  deprecate :aws_secret_access_key=, :none, 2025, 1
  deprecate :credentials_provider=, :none, 2025, 1

@mreinsch
Copy link
Contributor Author

@ericcj we've been using it like this since I opened the PR :-)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants