crossbario / crossbar

Crossbar.io - WAMP application router
https://crossbar.io/
Other
2.05k stars 274 forks source link

Crossbar has no changelog #685

Closed azazel75 closed 8 years ago

azazel75 commented 8 years ago

Today i stumbled across a disrupting change in the crossbar / AuthobahnPython codebase that surfaced with the 0.13 release: disclose_me=True member of CallOptions and PublishOptions classes has been obliterated and it has been refactored/ moved somewhere else that i have yet to find.

disclose_me together with the details member of RegisterOptions and SubscribeOptions are the key to being able to indentify and contextualize the caller/event emitter and this has been done without a warning, a mention in a changelog or in a mail. Even the page in pypi or the tag code are useless.

I really appreciate you for your enduring effort in publishing and maintaining crossbar and autobahn, but this situation where there is no changelog and/or no deprecation interval for obsolete features really seems like the project is managed by unexperienced devs from an external point of view, which i know it's not.

Please take at least 30 seconds when committing to compile a very brief changelog of the new features and bugs fixed and maybe publish it as the tag's comment..

If you want the community around crossbar to grow, you must know that the perception of such details really matters.

Yet again, thanks for all your work

meejah commented 8 years ago

Perhaps using something like https://github.com/hawkowl/towncrier would make adding a note about a feature/bug-fix easy enough that we actually do so...

oberstet commented 8 years ago

@azazel75 Sorry, yes, I agree. We really should have a proper changelog. And that is just part of the picture. We need a proper, synchronized (across the txaio, autobahn and crossbar packages) release management process too!

@meejah Towncrier sounds interesting! And it's made by @hawkowl ;) With towncrier, if I get it, I am supposed to leave smallish news fragment files, like myproject/newsfragments/850.feature, and the number has to be a ticket number. So that would require a strict discipline of "no change without ticket", and "one change per ticket" as a prerequisite, right? Because if we had that (and I am probably the culprit here;), and we had news fragments, then Towncrier would give us a changelog "for free"?

meejah commented 8 years ago

Personally, I think we'd need a "get out of jail free" card of X.feature or something so that if you didn't happen to have a ticket/issue number, you can still easily mention the new feature/bug etc. But, probably easy enough to add? Or, maybe there's value in "forcing" people to open a ticket, even if it's in hindsight/after-the-fact?

But yes, that's the idea as I understand it -- so that you don't get merge-conflicts in a single "changelog" or similar file.

oberstet commented 8 years ago

@azazel75 Ah, totally forgot. Regarding your actual issue. We've implemented the changes in the WAMP protocol specification regarding publisher/caller disclosure. This is now under strict a router side control. In Crossbar.io, you can configure disclosure like here https://github.com/crossbario/crossbarexamples/blob/master/disclose/.crossbar/config.json#L22 - Neither the caller nor the caller requests disclosure, but the procedure URI being called is configured (or not) for disclosure. This means, the API has changed across the Autobahn's as well as the configuration of Crossbar.io has changed.

azazel75 commented 8 years ago

@oberstet thank you, i figured it out with the help of @meejah, although i must say i'm not completely happy with the change... disclose_me/details was a way to establish a contract between the caller/publisher and the called/subscriber and i could setup all this without having to deal with service's configuration. Now probably this change would need me to setup a check that will warn the tampering sysadmin that he has changed the roles setup and has deleted a single line of the conf that makes all the system fragile.

I would have preferred having it as an alternate way of setting this detail, not the new exclusive, from code to service configuration way to do that.

I can say more... I can think to the dynamic authenticator as a better place to define this, per role...The authenticator approves the peer, but can enforce disclosure.. not bad. I would prefer it versus a change in a static file (but i haven't had a look at the code that implements it, maybe it's there already).

But all this brings up really another time how the change has been made, for me. Using another workflow for such disrupting changes would have made all this more clear (because maybe of the sprung up discussion on the ml or here) and the (dev-)users more aware. I mean a workflow like:

1) implement the new api/conf change as an alternative way of achieving the same goal; 2) at the same time, setup a DeprecationWarning when someone still uses CallOptions.disclose_me; 3) after one or two releases, drop the old behavior from the code.

Between 2 and 3, interested users can come to you guys to discuss the matter and so on...

my two cents ;-)

hawkowl commented 8 years ago

re towncrier: it's currently unbudging on the ticket number existing, because it's a rewrite of the Twisted code -- making it not require that should be easy enough, though.

hawkowl commented 8 years ago

@oberstet @meejah I've pushed up towncrier 16.1, which supports non-numeric ids. You can also have multiple categories per number -- eg. if you fix a bug and deprecate the broken code, you can do say "844.bugfix" and "844.removal".

But yes, with this, you get the changelog for free -- you just set up the config which tells it where to look, and you then go "towncrier", and it does this (the version change was manual, towncrier doing that is out of scope, that's incremental's duty): https://github.com/hawkowl/towncrier/commit/70c9c02ae25dfbe578dc15030f7c7829d7ba7640

oberstet commented 8 years ago

Lets go with towncrier and issue numbers. If I want to produce a changelog entry, it's usually a non-trivial change, and that should have an issue anyway. And numbers still seems to be easier to look over compared to subject titles.

I think we should be fine by using only 4 types of news fragment files:

where N is a GitHub issue number.

This allows us to produce a list of changes in terms relevant (namely, visible) to users: what functionality / behavior has been added, fixed, removed or documented since last release.

And that is addressing the original itch.

A minor thing, but given above usage, I'd prefer the file names like 850-add.md, 850-fix.md, 850-rem.md and 850-doc.md and all of these fragments in a crossbar/changes directory. Plus being able to use Markdown syntax in fragments, and have the final changelog be composed as Markdown would also rock;)

oberstet commented 8 years ago

How would that play together with semver?

E.g. if all changes are reflected in PRs which include a newsfragment of appropriate type(s), then the product version number under semver can be computed for the current changelog head after a PR got merged.

If there was any newsfragment of type "removal" merged since last version bump, then bump major.

Sidenote: there might have been newsfragments (and hence changes) of types "feature" or "bugfix" as well, but that doesn't make the minor and patch numbers increment?

If there was any newsfragment of type "feature" merged, but not "removal", then bump minor version.

If there were only newsfragments of type "bugfix" merged, then bump patch version.

But the version isn't usually bumped for each and every PR merged, but only "when one does a release", right? When exactly is a release made?

meejah commented 8 years ago

IIUC, the usage of towncrier would be part of the release-process -- it takes all the files in e.g. crossbar/changes/*, builds a change-log entry for that release, and then git rm's all those files. So, we could run a thing that automagically bumps the version right before towncrier runs based on the logic above (which sounds fine to me).

Ideally, this would be by automatically making a pull-request, so that "the person doing the release" can review it (including the deleted files, new changelog and version-bump) and just push the Big Green Button if all looks well.

So, a script could look like:

  1. git pull
  2. python scripts/bump-version.py, which would example crossbar/changes
  3. git checkout -b release-X.Y.Z
  4. towncrier which would build a changelog and git rm everything in crossbar/changes
  5. git add add the changelog
  6. git commit -m "release X.Y.Z"
  7. git push
  8. create pull-request (surely this can be scripted?)

This could then all even happen via some unprivileged account and the only thing needing commit access is the person who reviews the PR and merges (and then does other release-stuff, e.g. tag etc).

oberstet commented 8 years ago

We are using Towncrier now

azazel75 commented 6 years ago

Any progress in this closed but not resolved bug?

oberstet commented 6 years ago

pls see https://github.com/crossbario/crossbar/blob/master/docs/pages/ChangeLog.md#crossbario-17101-2017-10-31