Skip to content

chore: Bump CRT version to 0.51.0 from 0.48.0 #925

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

Closed
wants to merge 2 commits into from

Conversation

sichanyoo
Copy link
Contributor

@sichanyoo sichanyoo commented Apr 28, 2025

Corresponding aws-sdk-swift PR: awslabs/aws-sdk-swift#1927

Issue #

2819

Description of changes

  • CRT 0.51.0 removes FileHandle's conformance to IStreamable which used to contain 3 methods, one of which is length(). The length() function was the only thing we were using; so conformance was removed by CRT and that method is moved to SDK side.
  • isEndOfStream() is added as corresponding change to CRT-side bug fix. The fix addresses CRT HTTP client issue where it used to send empty frame after every nonempty data frame in HTTP/2 connection.

Scope

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link
Contributor

@jbelkins jbelkins left a comment

Choose a reason for hiding this comment

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

1 Q inline

@@ -109,4 +109,8 @@ public class StreamableHttpBody: IStreamable {
return nil
}
}

public func isEndOfStream() -> Bool {
return false
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the rationale for this always being false?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rationale was provided in CRT side here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After discussion with Waqar realized it has to have actual impl in SDK side; updated here: 55dd83c

@jbelkins
Copy link
Contributor

1 Q inline

(also see integration test failures in aws-sdk-swift)

@sichanyoo
Copy link
Contributor Author

Version bump to go out with #920, as there exists a bug in 0.51.0 and fix for the bug was added by rolling forward to 0.52.1 due to branch mgmt complications.

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.

3 participants