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

Inconsistent format for message IDs #37

Open
lgrahl opened this issue Sep 25, 2017 · 5 comments
Open

Inconsistent format for message IDs #37

lgrahl opened this issue Sep 25, 2017 · 5 comments

Comments

@lgrahl
Copy link
Contributor

lgrahl commented Sep 25, 2017

DeliveryReceipt.message_ids contains a list of IDs in bytes whereas the various send methods of the Connection instance usually return a hex-encoded ID. This makes it really annoying to handle.

We should unify the format. My personal tendency is to use bytes. This would definitely require a major version increase because it breaks API. Any thoughts, @dbrgn?

@lgrahl
Copy link
Contributor Author

lgrahl commented Sep 25, 2017

Supplement: I believe the reason why I did this was that it's harder to read byte representations in the CLI, so maybe the CLI should still print hex-encoded IDs.

@lgrahl
Copy link
Contributor Author

lgrahl commented Sep 25, 2017

(Let's make sure to remove the deprecated DeliveryReceipt.ReceiptType.user_ack alias when releasing a new major version.)

@dbrgn
Copy link
Contributor

dbrgn commented Sep 26, 2017

I'd go with the type returned by the API. If the API returns hex values, we could keep them in hex, it's a bit easier to handle. But converting everyting to bytes would be fine too, as long as it' sconsistent :)

@lgrahl
Copy link
Contributor Author

lgrahl commented Sep 26, 2017

I'd go with the type returned by the API.

Well, the API just can't return bytes because HTTP. 😉

Another reason why bytes is better: Hex-encoding is ambiguous (because case doesn't matter) which is an issue when comparing two IDs.

@dbrgn
Copy link
Contributor

dbrgn commented Sep 26, 2017

Ok, go for bytes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants