jasonrbriggs / stomp.py

“stomp.py” is a Python client library for accessing messaging servers (such as ActiveMQ or RabbitMQ) using the STOMP protocol (versions 1.0, 1.1 and 1.2). It can also be run as a standalone, command-line client for testing.
Apache License 2.0
491 stars 167 forks source link

Is 8.1.1 a legitimate release? #431

Closed AaronDMarasco closed 4 months ago

AaronDMarasco commented 4 months ago

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 commented 4 months ago

Yes it is. Just a missed git push.

AaronDMarasco commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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 a6a30652ef2eb1bce3ecfd7c654fb6f664cd4917 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 commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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

For completeness: