sbtinstruments / aiomqtt

The idiomatic asyncio MQTT client, wrapped around paho-mqtt
https://sbtinstruments.github.io/aiomqtt
BSD 3-Clause "New" or "Revised" License
393 stars 71 forks source link

Switch to poetry and rename to aiomqtt #210

Closed empicano closed 1 year ago

empicano commented 1 year ago

Finally found some time for this! @frederikaalund, @JonathanPlasse, if you could look over the pyproject.toml that would be great, in particular, I'm not sure if I configured the poetry-dynamic-versioning right.

I published it on Test PyPI: https://test.pypi.org/project/aiomqtt-test/0.16.1.post59

In this PR I still want to update all references from asyncio-mqtt to aiomqtt in the documentation and the code. When the PR is merged I would release 0.17.0 both under the aiomqtt and the asyncio-mqtt names. For asyncio-mqtt I would add a log warning explaining the rename and the migration and publish it manually. Then I would change the name of the repository on GitHub (which also updates the URL of the documentation).

Contrary to what I proposed in #202 I would make this the last release for asyncio-mqtt. The rename doesn't break anything for asyncio-mqtt users that want to stay with the old name, and the migration should be straightforward. Also, I admit that I don't know how to publish to two names automatically and expect this to be unnecessarily complex.

Let me know what you think! 😊

JonathanPlasse commented 1 year ago

I am busy this week. It will take some time for me to review this PR.

empicano commented 1 year ago

I'd like to make our CONTRIBUTING.md a bit easier by going for GitHub's "Scripts to rule them all". We can then also use these scripts inside the GitHub workflows (see scripts/setup inside the docs workflow). What do you think about that?

empicano commented 1 year ago

Does any of you have experience with using poetry-dynamic-versioning inside the GitHub workflow and can point me to a code snippet? I'm a bit stuck as to how to do this. I also found this GitHub action that takes the version from the tag on GitHub: https://github.com/marketplace/actions/pypi-poetry-publish

frederikaalund commented 1 year ago

I'd like to make our CONTRIBUTING.md a bit easier by going for GitHub's "Scripts to rule them all". We can then also use these scripts inside the GitHub workflows (see scripts/setup inside the docs workflow). What do you think about that?

Great idea! πŸ‘ That is one of the more difficult things to maintain manually. PRs usually forget about it or find that its a barrier for contribution.

Does any of you have experience with using poetry-dynamic-versioning inside the GitHub workflow and can point me to a code snippet?

I'm afraid not. I tried it way back (in another project) but never got a workflow up around it.

empicano commented 1 year ago

@JonathanPlasse what does this JUnit stuff here and in the test workflow do? Do we still need this?

JonathanPlasse commented 1 year ago

Yes, we still need this. It is used to have error messages in the PR code where there are mypy or pytest errors.

empicano commented 1 year ago

pre-commit is really starting to annoy me, I just want to test the workflows without pulling in the changes from main

JonathanPlasse commented 1 year ago

You can use workflow_dispatch to manually run the workflows.

frederikaalund commented 1 year ago

Great to see progress on this issue. πŸ‘ Keep up the good work! :)

I have some time the next couple of days to contribute as well. Let me know if you need something from my side.

empicano commented 1 year ago

You can use workflow_dispatch to manually run the workflows.

Maybe I'm not seeing something, but I can only run the workflows from main there.

I'm a bit frustrated with changing the tooling to poetry. This takes a lot more time than I anticipated. Can we remove pre-commit in favor of a script (see scripts/check that runs ruff and mypy) that we can run locally and that runs on each push and PR? I feel that pre-commit is really slowing me down (e.g. see above, there's no reason this should block the other workflows, but also I want to run the checks when I want, not necessarily on each commit).

Related, the idea to "enable the strict rules and then help contributors fix them" from #217 doesn't work if pre-commit forces contributors to fix things before they commit.

If we remove pre-commit I think I can finish this PR with some oversight from Jonathan on the JUnit stuff. Otherwise, I have to pass this on to someone else because I'm missing the time. Independently, I can still update the docs to the new name.

frederikaalund commented 1 year ago

It's fine with me to disable pre-commit to get the move to poetry done. πŸ‘ If I had to choose between the two, then poetry wins no doubt. :) We have all the checks server-side (via GitHub workflows) anyhow.

The idea with pre-commit is that you run the same checks locally first to avoid the wait time (i.e., the wait time for the GitHub workflows to complete). :) That is a nice-to-have feature. We can disable it for now to get things moving.

On a side note: Now that we have a poetry-based local environment, we can use that for pre-commit. That is, use repo: local in the .pre-commit-config.yaml for all python-based "hooks". See an example of that type of config here: https://github.com/sbtinstruments/cyto/blob/master/.pre-commit-config.yaml This way, we make pyproject.toml file for all our python dependencies (even dev dependencies). Otherwise, pre-commit makes separate envs for it's hooks (and different versions).

frederikaalund commented 1 year ago

Also, there is always git commit --no-verify if you wan to skip the pre-commit hooks. They are optional in that sense. We can't enforce the client to apply said hooks. :)

empicano commented 1 year ago

there is always git commit --no-verify if you wan to skip the pre-commit hooks.

That's a good tip, thanks πŸ™‚ We should definitely add that to the CONTRIBUTING

The idea with pre-commit is that you run the same checks locally first to avoid the wait time

Right, that's a good thing, I just prefer to run those checks when I want it, and not have them forced on me when I want to commit πŸ˜„ Anyways, if I interpret it right, you're for keeping pre-commit as a workflow (which is what is blocking me from testing the updated workflows atm), so I'll focus on replacing all occurrences of "asyncio-mqtt" with "aiomqtt" in the docs, and let someone else finish the move to poetry πŸ˜‹

frederikaalund commented 1 year ago

Anyways, if I interpret it right, you're for keeping pre-commit as a workflow (which is what is blocking me from testing the updated workflows atm), so I'll focus on replacing all occurrences of "asyncio-mqtt" with "aiomqtt" in the docs, and let someone else finish the move to poetry πŸ˜‹

Ah, I actually didn't realize that we use pre-commit as a driver for our GitHub workflows. πŸ˜…

Well, let's get the rename done with. Then we'll tackle the move the poetry in another PR. :) πŸ‘

frederikaalund commented 1 year ago

I just looked the GitHub workflow files. I can't see any reference to pre-commit there. Am I missing something?

empicano commented 1 year ago
Screenshot 2023-05-19 at 21 23 21

that we use pre-commit as a driver for our GitHub workflows.

Yes, and who knows why, but pre-commit blocks the execution of other workflows because there are merge commits (even though this is still a draft). I can't look into the project settings, but I suspect that pre-commit is listed as an authorized app there and that this is how it has access.

JonathanPlasse commented 1 year ago

This is a GitHub app which must be disabled.

JonathanPlasse commented 1 year ago

You should be able to disable it Here

empicano commented 1 year ago

Ping @frederikaalund in case this got lost πŸ˜‰

frederikaalund commented 1 year ago

Ping @frederikaalund in case this got lost πŸ˜‰

Sorr about that! πŸ˜… It did get lost among my other work.

I just suspended the pre-commit app. Let me know if you need something more! πŸ‘

empicano commented 1 year ago

@JonathanPlasse, any idea why the PyPy tests keep failing? The other tests work fine, and I can't wrap my head around the Directory /opt/hostedtoolcache/PyPy/3.9.16/x64/lib/pypy3.9 for cffi does not seem to be a Python package error.

(Solved by pinning the snok/install-poetry action to its highest version.)

empicano commented 1 year ago

This is finally finished! @frederikaalund, can you rename the repository on GitHub? I've updated all the links and mentions to the new name, that's the only thing missing.

@mossblaser, thank you again for generously liberating the aiomqtt name! πŸŽ‰ 😎

frederikaalund commented 1 year ago

This is finally finished! @frederikaalund, can you rename the repository on GitHub?

Done!

Thank you @empicano for (yet another) great contribution to asyncio-mqtt! πŸ˜„πŸ‘

And thanks to @mossblaser for the aiomqtt name. :) Makes me nostalgic, because mossblaser's aiomqtt was my original motivation for asyncio-mqtt.

mossblaser commented 1 year ago

Hah, glad to hear it led to less hacky things!

I'd welcome a suitable message to be inserted into my old aiomqtt repo's readme pointing people to your project if you'd be happy to suggest some suitable words?

On Tue, 13 Jun 2023, at 6:29 PM, Frederik Aalund wrote:

This is finally finished! @frederikaalund https://github.com/frederikaalund, can you rename the repository on GitHub?

Done!

Thank you @empicano https://github.com/empicano for (yet another) great contribution to asyncio-mqtt! πŸ˜„πŸ‘

And thanks to @mossblaser https://github.com/mossblaser for the aiomqtt name. :) Makes me nostalgic, because mossblaser's aiomqtt was my original motivation for asyncio-mqtt.

β€” Reply to this email directly, view it on GitHub https://github.com/sbtinstruments/aiomqtt/pull/210#issuecomment-1589739050, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABPH2UDEL2RHTWG3RNLVGLXLCPRHANCNFSM6AAAAAAXSHKCI4. You are receiving this because you were mentioned.Message ID: @.***>

frederikaalund commented 1 year ago

I'd welcome a suitable message to be inserted into my old aiomqtt repo's readme pointing people to your project if you'd be happy to suggest some suitable words?

I already like the current You might prefer aiomqtt section. :) Thank you for that, btw.