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

Is 8.1.1 a legitimate release? #431

Closed
AaronDMarasco opened this issue Apr 11, 2024 · 7 comments
Closed

Is 8.1.1 a legitimate release? #431

AaronDMarasco opened this issue Apr 11, 2024 · 7 comments

Comments

@AaronDMarasco
Copy link
Contributor

It was posted to PyPI 5 days ago, but no reference in the GitHub changelog nor changes in the repo for 10 months?

With lots of PyPI maliciousness in the news somewhat recently, I'm concerned.

@jasonrbriggs
Copy link
Owner

Yes it is. Just a missed git push.

@AaronDMarasco
Copy link
Contributor Author

OK thanks. Not a fan of "Change version from tuple to string" in a "patch version" because that broke all our code, which was what had me digging into all this in the first place...

@jasonrbriggs
Copy link
Owner

jasonrbriggs commented Apr 13, 2024

Apologies my bad. Yes that really warrants a major version bump since its a backwards incompatible change...

Starting to wonder if I should pull back these releases and re-issue properly with a corrected version...

@AaronDMarasco
Copy link
Contributor Author

AaronDMarasco commented Apr 13, 2024

Starting to wonder if I should pull back these releases and re-issue properly with a corrected version...

The argument goes both ways - it's got a leading underscore, so technically private. But it's also a de-facto dunder that can be considered part of the public API.

IIRC I had our requirements as < 9 and was surprised when coworkers started coming to me saying stuff is broken.

I was going to send a PR your way with it reverted, but then saw that a6a3065 didn't just change it (tuple -> str) but you now have an automated process available to keep it up-to-date; also a worthy endeavor.

My personal vote is keep it as tuple; I don't have access to my source right now, but I know we do lots of compares, IIRC major versions 4 and 7 had a lot of changes we needed in the way data is being handled. But ours is also a legacy source base, and this is OSS so it's obviously your call. I inherited the earliest checks; I don't know if somebody internal to the team came up with those comparisons or if that's a way you've documented it in the past for users to check.

I also don't know enough about binary websockets to determine if that is worth a major bump anyway, independent of this conversation.

@jasonrbriggs
Copy link
Owner

Personally not enthused about changing it back to a tuple because the version string is more consistent with other projects - and it works with poetry's auto version bump, which I prefer. I don't mind adding a function to flip it back to a tuple if preferred (something like stomp.tuple_version() to change it from a string back to a tuple) -- but seems kind-of pointless given you'd have to make a change to call that anyway... but I don't know how many references you have to the tupled version, so maybe it would provide some value?

@AaronDMarasco
Copy link
Contributor Author

Honestly, I just locked it at 8.1.0 and if for some reason we ever need to update again, I'd go thru and remove all the checks and force it to a later version. I believe some of the checks are from when we were trying to support RHEL6, 7, and 8 simultaneously.

@AaronDMarasco
Copy link
Contributor Author

IIRC major versions 4 and 7 had a lot of changes we needed in the way data is being handled

For completeness:

  • Before 4.1.20 there was an explicit call to conn.start() after creation but before conn.connect() call.
  • 7.0.0 and after on_message now has no body argument (see Original headers #309), so need to extract headers and body individually from the incoming frame

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

No branches or pull requests

2 participants