search

mushorg / conpot

ICS/SCADA honeypot
GNU General Public License v2.0
1.22k stars 413 forks source link

Let Tox deduce the Python version #488

Closed srenfo closed 2 years ago

srenfo commented 4 years ago

Partial fix for #486. Makes tox.ini version-agnostic. Also makes sure we always test the installed version.

xandfury commented 4 years ago

Please see comment on #486 :slightly_smiling_face:

srenfo commented 4 years ago

This makes tox version-agnostic, for lack of a better term. For example, the current Ubuntu 20.04 ships with Python 3.8 only. tox would not run unless patched like this.

So this eases the pain of #486 but certainly doesn't make it go away.

Of course you also cede some control because you're no longer setting the version explicity. But you should be doing that in your Travis config or such. The risk is that someone may run tox with an unsupported version of Python and open a corresponding issue. python_requires in setup.py should catch that, but I'm not sure.

xandfury commented 4 years ago

Precisely this :slightly_smiling_face: :

Of course you also cede some control because you're no longer setting the version explicity.

We run tox for just tests. And tests are only run by devs who are starting development. We really had a strong discussion around this in 2018. It was agreed that we hardcut conpot for particular pinned versions of python.

As you might've guessed migrating from python2 was not trivial for us. Which is why we are extra careful about this. :slightly_smiling_face:

But we can wait for others to drop their opinion on this.

glaslos commented 2 years ago

@xandfury how would you feel about this PR now that we have a fixed python version in the GitHub Action?

xandfury commented 2 years ago

Looks good! Just need to fix the conflicts then 🙂