Better use of the docker cache

[obilodeau]

docker: Code is added in the runtime image now to avoid costly rebuilds

Still needed to compile the rle module in the compile image which required some tests and wizardry.

slim image change coming soon but I wanted to see it in GitHub actions first. 👀

Fixes #217

[obilodeau]

Maintaining the requirements.txt file is going to be less of a hassle than previously thought because CI or DockerHub will blow-up all the time.

[Res260]

Shouldn't we still lock to minor version since patch versions are usually bug/security fixes?

[obilodeau]

Shouldn't we still lock to minor version since patch versions are usually bug/security fixes?

I don't understand. Since it's not really important to the change we can discuss it on keybase.

[obilodeau]

Shouldn't we still lock to minor version since patch versions are usually bug/security fixes?

I don't understand. Since it's not really important to the change we can discuss it on keybase.

I'm sorry when I said this I thought you were talking about the change to 0.4.2.dev0. Clearly you are talking about the content of requirements.txt. This I'm open to discuss here. Sorry for my confusion.

[obilodeau]

So the way I feel about the requirements.txt files is as follows:

We won't change our install documentation. Meaning people will still install using setup.py and not requirements.txt. setup.py doesn't define precise version numbers. So setup.py is our package.json and requirements.txt is our package-lock.json (where exact versions are defined and dependency tree is explicit). GitHub will automatically notify us of CVE issues and so we will update the versions and this will build new docker images. If we don't give precise version numbers our docker images might carry vulnerable versions.

I'm on the fence on this but I think having exact versions defined would be slightly better. What do you think?

[Res260]

I'm okay with exact version numbers. I just find it a bit odd that setup.py doesn't have version restrictions.

Take this scenario for example:

I want to implement a feature for PyRDP. This features requires to use a feature in a dependency. Said feature is new in the latest version. If I implement said feature, it will work on my machine, and CI will pass. However, the docker build will be broken, because it has an old version of the dependency.

This would mean that the developper of the feature would need to make sure to test separately on Docker everytime a new feature using a dependency is introduced, which seems like an unnecessary overhead.

Before the requirements.txt file, this would have been less of a problem, because both the source and dockerfile would use the latest versions of the dependencies, but now they will act differently.

I think if we're going to lock requirements.txt, we should lock setup.py too (at least to minor versions). Does it make sense?

[obilodeau]

I think if we're going to lock requirements.txt, we should lock setup.py too (at least to minor versions). Does it make sense?

I'm not against it. But it's a time-consuming thing to do if we want to do it right. Let's go through the list. appdirs should we pin 1.4.x or 1.x? I can't really rely on semver because many library authors don't do it right. I would need to load their homepage and evaluate by reading the changelog. Another example is Twisted. We were at 19 a few months ago but they released 20 recently and our code works perfectly with it. Pinning to 19 would have held us back until we re-evaluate. Enforcing 19 or 20 might prevent us from being deployed in some environments where older versions are required and pyrdp probably works well with 16 (if it exists).

I can do a "best effort" quick first pass and see what it looks like.

[obilodeau]

This is worth reading: https://packaging.python.org/discussions/install-requires-vs-requirements/

Under install_requires:

It is not considered best practice to use install_requires to pin dependencies to specific versions, or to specify sub-dependencies (i.e. dependencies of your dependencies). This is overly-restrictive, and prevents the user from gaining the benefit of dependency upgrades.

Under "Requirements file":

Whereas install_requires requirements are minimal, requirements files often contain an exhaustive listing of pinned versions for the purpose of achieving repeatable installations of a complete environment.

[Res260]

Looks good. PyOpenSSL also uses Calendar Versionning (https://calver.org/#twisted)

Its versioning scheme has spread to related projects, including Klein, Treq, and even one of Twisted's dependencies, PyOpenSSL.

[obilodeau]

Just adjusted PyOpenSSL as well.