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

Bump aws-* dependencies from >= 0.10 to >= 0.53 #108

Merged
merged 3 commits into from
Jan 31, 2023

Conversation

Xtansia
Copy link
Collaborator

@Xtansia Xtansia commented Jan 16, 2023

Description

Bump aws-* dependencies from >= 0.10 to >= 0.53
Credential types were split from aws-types into aws-credential-types

Issues Resolved

Resolves #106

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Credential types were split from `aws-types` into `aws-credential-types`

Signed-off-by: Thomas Farr <[email protected]>
Signed-off-by: Thomas Farr <[email protected]>
@codecov-commenter
Copy link

codecov-commenter commented Jan 16, 2023

Codecov Report

Merging #108 (c8658f6) into main (461ea93) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #108   +/-   ##
=======================================
  Coverage   42.28%   42.28%           
=======================================
  Files          50       50           
  Lines       28086    28086           
=======================================
  Hits        11877    11877           
  Misses      16209    16209           
Flag Coverage Δ
integration 42.28% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
opensearch/src/auth.rs 0.00% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@dblock
Copy link
Member

dblock commented Jan 17, 2023

So is this a breaking change?

@Xtansia
Copy link
Collaborator Author

Xtansia commented Jan 17, 2023

So is this a breaking change?

Yeah unfortunately, as due to the nature of the api it pushes that higher dependency onto the consumer (if they're using awssigv4). But we could maybe handwave it into a minor bump due to aws-* being treated as unstable and it being unrealistic to major bump the client everytime they change something.

However I am going to think a bit about how this is exposed to clients (and my comments on #96) to see if we could nicely wall it off further.

@dblock
Copy link
Member

dblock commented Jan 20, 2023

Handwave it.

@VachaShah
Copy link
Collaborator

@Xtansia I could not find sample code for SigV4 in the user guide. Am I missing something or does it have to be added?

@dblock dblock merged commit 892cb80 into opensearch-project:main Jan 31, 2023
@dblock
Copy link
Member

dblock commented Jan 31, 2023

I have docs as part of #96, but we have more problems.

@dblock
Copy link
Member

dblock commented Jan 31, 2023

@Xtansia with this upgrade I am getting the following:

---- src/lib.rs - (line 342) stdout ----
error[E0277]: the trait bound `opensearch::auth::Credentials: From<SdkConfig>` is not satisfied
  --> src/lib.rs:359:61
   |
19 | let transport = TransportBuilder::new(conn_pool).auth(creds.try_into()?).build()?;
   |                                                             ^^^^^^^^ the trait `From<SdkConfig>` is not implemented for `opensearch::auth::Credentials`
   |
   = help: the trait `From<ClientCertificate>` is implemented for `opensearch::auth::Credentials`
   = note: required for `SdkConfig` to implement `Into<opensearch::auth::Credentials>`
   = note: required for `opensearch::auth::Credentials` to implement `TryFrom<SdkConfig>`
   = note: required for `SdkConfig` to implement `TryInto<opensearch::auth::Credentials>`

@Xtansia Xtansia deleted the update/aws-types branch January 31, 2023 20:14
@Xtansia
Copy link
Collaborator Author

Xtansia commented Jan 31, 2023

@Xtansia with this upgrade I am getting the following:

---- src/lib.rs - (line 342) stdout ----
error[E0277]: the trait bound `opensearch::auth::Credentials: From<SdkConfig>` is not satisfied
  --> src/lib.rs:359:61
   |
19 | let transport = TransportBuilder::new(conn_pool).auth(creds.try_into()?).build()?;
   |                                                             ^^^^^^^^ the trait `From<SdkConfig>` is not implemented for `opensearch::auth::Credentials`
   |
   = help: the trait `From<ClientCertificate>` is implemented for `opensearch::auth::Credentials`
   = note: required for `SdkConfig` to implement `Into<opensearch::auth::Credentials>`
   = note: required for `opensearch::auth::Credentials` to implement `TryFrom<SdkConfig>`
   = note: required for `SdkConfig` to implement `TryInto<opensearch::auth::Credentials>`

@dblock Based on the error my best guess is either not having the aws-auth feature enabled on the crate, or mismatched version for the aws-config crate, Cargo.toml should be something like:

[dependencies]
opensearch = { git = "https://github.com/opensearch-project/opensearch-rs.git", features = ["aws-auth"] }
aws-config = "0.53"

@dblock
Copy link
Member

dblock commented Jan 31, 2023

Thanks, I solved it like so.

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.

[FEATURE] Does not build with newest aws rust sdk (aws-types = "0.53")
4 participants