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

THRIFT-5653: UUID should use value 16 in Swift binary protocol #2717

Merged

Conversation

kinoroy
Copy link
Contributor

@kinoroy kinoroy commented Oct 20, 2022

Removes non-standard utf8 type with raw value 16 and replaces uuid raw value with 16.
Brings Swift in compliance with spec:

* `UUID`, encoded as `16`

This is technically a breaking change by removing the utf8 type, however this type was non-standard and not documented in the spec.

This causes new failing crosstests with Swift-Java with the binary protocol due to Java not yet being updated to match the spec.

  • Did you create an Apache Jira ticket? (not required for trivial changes)
  • If a ticket exists: Does your pull request title follow the pattern "THRIFT-NNNN: describe my issue"?
  • Did you squash your changes to a single commit? (not required, but preferred)
  • Did you do your best to avoid breaking changes? If one was needed, did you label the Jira ticket with "Breaking-Change"?
  • If your change does not involve any code, include [skip ci] anywhere in the commit message to free up build resources.

Removes non-standard utf8 type with raw value 16 and replaces uuid raw value with 16
Copy link
Member

@fishy fishy left a comment

Choose a reason for hiding this comment

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

Thank you for doing this!

(the following can be fixed in new PR(s) instead) travis has been failing with this swift error:

/thrift/src/lib/swift/Sources/TSocketServer.swift:83:60: error: type 'CFSocketError' (aka 'Int') has no member 'success'
      if CFSocketSetAddress(sock, cfaddr) != CFSocketError.success { //kCFSocketSuccess {
                                             ~~~~~~~~~~~~~ ^~~~~~~

since it looks like you are familiar with swift (😅), do you mind taking a look at that?

@kinoroy
Copy link
Contributor Author

kinoroy commented Oct 20, 2022

@fishy yes I can look at that, the reason is that Swift on Linux is not as stable as it is on macOS yet, they sometimes make breaking updates to the foundation library on Linux. I will look to see if we can standardize the version of Swift we use across Travis and Github actions.
Edit: tracking in https://issues.apache.org/jira/browse/THRIFT-5657

@jimexist jimexist merged commit baa0daa into apache:master Oct 21, 2022
@kinoroy kinoroy deleted the THRIFT-5653-UUID-should-use-value-16-in-swift branch October 23, 2022 02:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants