a couple of packaging fixes

[dimbleby]

• use pep 508 markers to statically express dependencies
• remove deprecated use of setup.py as a command line tool

I noticed in passing that this claim in setup.py

 # wsaccel does not provide wheels: https://github.com/methane/wsaccel/issues/12

has not been true for some time, but left it alone

[oberstet]

great, thanks for contributing! some notes: IMO the most invasive change should be checked, mentioned/discussed - the removal of setuptools (the package dep). I am not sure where it is used/needed, probably not only for installation ...

[oberstet]

has not been true for some time, but left it alone

ah, great! missed that. fwiw, np removing deprecated notes/comments like this!

remove deprecated use of setup.py as a command line tool

is that "official"? if not, I don't care, as long as it works, don't touch would be my preference ..

[dimbleby]

https://packaging.python.org/en/latest/discussions/setup-py-deprecated/

I did not remove the package dependency on setuptools (iirc it is needed because of use of pkg-resources)

[oberstet]

ok, thanks a lot for digging and for the link!

this link should be in the doc comments somewhere .. so that the any change here can be traced back to the reason that triggered the change

However, python setup.py and the use of setup.py as a command line tool are deprecated.

"however" as in: depending on setuptools is not deprecated or discouraged

"the problem is: this PR does both (remove use of setup.py and setuptools)" - well, actually this is WRONG;) sorry, missed it. anyways, we should IMO definitely keep the setuptools package (for now) ...

[dimbleby]

Setuptools is a default build requirement and does not need to be set explicitly for installation. The proof of that pudding is the successful wheel builds.

This project also uses setuptools as a runtime requirement, and I have left this alone. The proof of that pudding should be in passing unit tests - I don't think the changes here can be responsible for the CI failures?

[oberstet]

This project also uses setuptools as a runtime requirement, and I have left this alone.

yes indeed, deliberately / by design (currently), so leaving it alone is appreciated, thanks!

The proof of that pudding should be in passing unit tests

yes, I agree, and it (ABPy without the PR) might not have coverage =( well .. "should" .. I definitely agree! that would allow easy checking of why setuptools is a run-time dep anyways - which I forgot, but there was "something" .. I seem to remember

I don't think the changes here can be responsible for the CI failures?

nope. it is related to breaking changes in asyncio / asyncio-test related to loop handling :(

the flake8 and twisted tests run all fine, so this strongly suggests this PR is good .. I am just not sure how to handle that: should we

a) wait until the aio thing is fixed after which eg this PR should run 100% green (after merging any changes from the former into this PR), OR should we b) merge this PR right aways as it is "likely" good even though not 100% green (because of the unrelated aio stuff)

?

@meejah any opinions how to move on?

also, complaining: why doesn't GH allow me to request reviewers anymore? did they break their shit, or what's going on? anyways. stuff.

[oberstet]

for comparison, this PR also triggers the aio issue (but not the other things failing here ...) https://github.com/crossbario/autobahn-python/pull/1630

[dimbleby]

look like the same failures to me - just variation in which checks run and fail first, and which get cancelled as a result?

[oberstet]

Yeah , could be.

David Hotham @.***> schrieb am So., 24. März 2024, 14:44:

look like the same failures to me - just variation in which checks run and fail first, and which get cancelled as a result?

— Reply to this email directly, view it on GitHub https://github.com/crossbario/autobahn-python/pull/1632#issuecomment-2016814750, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABY67C2DT6Y4DBRTWV2FI3YZ3KEJAVCNFSM6AAAAABFEWD572VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMJWHAYTINZVGA . You are receiving this because you commented.Message ID: @.***>

[meejah]

@meejah any opinions how to move on?

"In general" moving away from setup.py is a good thing. Best-practice now is to use "configuration only" for most tasks (e.g. via pyproject.toml). Unfortunately, there's now even more Python packaging tools available than last time I made a new project haha :cry: but there are also PEPs to make them play more-nicely together.

The changes here look plausible to me (but of course it's also less terrifying to merge when things are green :) )

[meejah]

Also, is there a bug specifically about the asyncio changes?

I assume that refers to the "event loop is closed" errors in the logs. It would be ideal to tie this back to the specific asyncio changes related to this...

[oberstet]

Also, is there a bug specifically about the asyncio changes?

I assume that refers to the "event loop is closed" errors in the logs. It would be ideal to tie this back to the specific asyncio changes related to this...

I think this the change in pytest-asyncio discussed here triggered the regression https://github.com/crossbario/autobahn-python/pull/1628#issuecomment-1974910401

[dimbleby]

@oberstet my real motivation in fixing the unit tests was to get this green and - hopefully - merged

[oberstet]

my real motivation ...

sure, I get that - it's fixed and merged! the minimum twisted version is also bumped, and it'll all be in the next autobahn release. thanks for contributing!

[dimbleby]

sounds good, thanks. Though I am slightly confused, this pull request is not yet merged?

[oberstet]

Though I am slightly confused, ...

sorry, I got confused ... merged that other PR ... which actually (now that you force pushed onto this PR) now also makes this one green ... so I (only) now merged this one as well;)

anyways, as @meejah notes, moving to configuration based setup etc is "welcome/progress" - and I hope there won't be lots of fallout;)