Skip to content

Allow psutil 6.0 in requirements - #473

Closed
hategan wants to merge 2 commits into
mainfrom
psutil_6
Closed

hategan wants to merge 2 commits into
mainfrom
psutil_6

Conversation

@hategan

@hategan hategan commented Jun 27, 2024

Copy link
Copy Markdown
Collaborator

Fixes #472

@Andrew-S-Rosen

Andrew-S-Rosen commented Jun 27, 2024 •

Copy link
Copy Markdown
Contributor

Thanks!

If I may comment here, it does not seem ideal to me that the requirements.txt file is being set up in this way. It means that every time there is a new release of psutil, libraries using psij-python as a dependency cannot upgrade or test against the latest psutil. This is what made me reach out in the first place, because dependabot was telling me I couldn't support psutil 6.0 in my package or in CI.

In general, I would not suggest putting a strict (==) or downward (<=) pin on any package dependencies unless there is good reason to (e.g. as a safety against major versions or known breaking changes that have yet to be resolved) so that it does not negatively impact code developers as well as users who may have certain requirements for packages in their given environment.

[It is also worth noting while we're on the topic that it seems Python best practices have shifted towards having pyproject.toml (or setup.py in psij-python's case since there is no pyproject.toml) include dependencies with as loose restrictions as possible, while having requirements.txt not included in the package requirements but be the exact (i.e. ==) versions used in CI for reproducibility. Of course, that is personal opinion and would require actively updating requirements.txt with something like dependabot.]

In any case, this PR technically closes #472, but the issue is going to crop up again each time psutil updates.

@hategan

hategan commented Jun 27, 2024

Copy link
Copy Markdown
Collaborator Author

If I may comment here, it does not seem ideal to me that the requirements.txt file is being set up in this way. It means that every time there is a new release of psutil, libraries using psij-python as a dependency cannot upgrade or test against the latest psutil. [...]
In any case, this PR technically closes #472, but the issue is going to crop up again each time psutil updates.

Thank you for bringing this up.

The way it is set up is largely intentional, the reason being that we would rather have to explicitly test new versions of a library than have users run into mysterious problems because one of the packages decided to make breaking changes in a minor update (something that has happened in the past - took quite a bit of effort to find).

We could make one choice or another. However, no matter what choice we make, it is impossible to ensure, given two packages that depend on some library, that the requirements of the two packages are both simultaneously met with a single version of that library and that they are both working correctly since they may simply be only working correctly with different versions of the library. Even the simple assumption that a future version of a library will necessary work as expected is, from a theoretical standpoint, not true.

So the question that remains is what do we shift to the user: the burden of having to troubleshoot potentially obscure internal incompatibilities between psij and a library, or the burden of submitting these feature requests. It's kinda hard to answer because, to be fair, we need to look at the expectation value of things, and it might very well be that the rate at which libraries versions (at the granularity we track) are increased is higher than the rate at which incompatibilities of the kind we dislike are introduced. And, also quite likely, those rates are different for different libraries.

I'll reach out to folks and see if we can acquire more wisdom in the matter.

@Andrew-S-Rosen

Andrew-S-Rosen commented Jun 27, 2024 •

Copy link
Copy Markdown
Contributor

Thanks for the reply and consideration.

The way it is set up is largely intentional, the reason being that we would rather have to explicitly test new versions of a library than have users run into mysterious problems because one of the packages decided to make breaking changes in a minor update (something that has happened in the past - took quite a bit of effort to find).

I don't believe this is at odds with my suggestion. If you run your CI to run tests on requirements.txt with strict version pins but have the package dependencies (i.e. those listed in setup.py or pyproject.toml) with minimal version restrictions, then you have the best of both cases. If you use dependabot or a similar platform, then your versions in requirements.txt will be tested and updated regularly so that you're always confident the code is working as expected when pip installed. If a breakage occurs, you can either patch and release or put a temporary version <= bound in setup.py/pyproject.toml until it's fixed, in which case you may need to add a >= bound.

Perhaps the issue with this logic is that you don't intend to mint releases at such a cadence?

Of course, you can choose to maintain the package however you deem most desirable! But I think it would be unfortunate to restrict users to specific versions even when incompatibilities do not necessarily exist.

@hategan

hategan commented Jun 27, 2024 •

Copy link
Copy Markdown
Collaborator Author

Thanks for the reply and consideration.

The way it is set up is largely intentional, the reason being that we would rather have to explicitly test new versions of a library than have users run into mysterious problems because one of the packages decided to make breaking changes in a minor update (something that has happened in the past - took quite a bit of effort to find).

I don't believe this is at odds with my suggestion. If you run your CI to run tests on requirements.txt with strict version pins but have the package dependencies (i.e. those listed in setup.py or pyproject.toml) with minimal version restrictions, then you have the best of both cases. If you use dependabot or a similar platform, then your versions in requirements.txt will be tested and updated regularly so that you're always confident the code is working as expected when pip installed. If a breakage occurs, you can either patch and release or put a temporary version <= bound in setup.py/pyproject.toml until it's fixed, in which case you may need to add a >= bound.

We need to specify, for each dependency, a range of versions for which we know a deployment of psi/j works correctly. For this purpose, there is no distinction between installing psij as a library in a larger project through pip install psij-python or setting up a local venv and doing pip install -r requirements.txt. We use a single source of truth for this, which happens to be requirements.txt (but it does not have to be). There is little purpose in having two different configurations here. If part of a larger deployment, we want things to be constrained such that they work. When building a new venv, pip automatically selects the latest dependencies satisfying the constraints, but we still want to satisfy those constraints. But there is little point in having a different tool selecting those latest versions explicitly.

The dependabot suggestion is great, because it prevents users from having to poke us to update things. I believe it can be configured to widen the current constraints, which is what we do anyway. And the resulting PRs would be automatically tested on the various clusters we test on, so it should be exhaustive.

Perhaps the issue with this logic is that you don't intend to mint releases at such a cadence?

Ideally, we do and it only takes a minute or so to make a release, and we can probably further automate that, but I think we want a "take a minute and think about it" step in that workflow.

Of course, you can choose to maintain the package however you deem most desirable! But I think it would be unfortunate to restrict users to specific versions even when incompatibilities do not necessarily exist.

We want reasonable confidence that incompatibilities do not exist. But I think we can achieve that with your dependabot suggestion. I'll try to set it up in the next few days.

@hategan

hategan commented Jul 1, 2024

Copy link
Copy Markdown
Collaborator Author

I'll try to set it up in the next few days.

Dependabot is up and running. I will close this PR since it is not relevant any more (the dependabot PR updating this library was merged).

@hategan hategan closed this Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature request: support for psutil 6.0

2 participants