migrate to pyproject + standard python bootstrap#16
Conversation
|
Is this ready for review? I'm a bit lost since you've requested review while CI was still failing, and now you've pushed more commits, I don't know what is this PR status. |
|
Yes, it is now ready for review. Was failing because of the nose directive and not adding the dependencies group to CI |
benoit74
left a comment
There was a problem hiding this comment.
See comments.
We also need some tests. At least a mostly stupid tests which ensures everything loads correctly on every supported Python versions. This is also important to allow to have a playground to add more tests as we see fit.
And the test workflow is also here to check we can build correctly Python package.
| "License :: OSI Approved :: GNU General Public License v3 or later (GPLv3+)", | ||
| ] | ||
| dependencies = [ | ||
| "boto3 == 1.37.38", |
There was a problem hiding this comment.
Since this is a package used a library in many projects, we need to use ranges here. Otherwise all projects using this package will have to use these specific versions, which is a maintenance nightmare.
Please check what is the reasonable minimum supported version to have sufficient APIs and use than as the lower end. For the upper end, this is more a matter of appreciation. In general upper end must at least be the next major (since a major is expected to break APIs). In some cases it should be the next minor because the dependency is not really respecting semantic versioning. Sometimes we might want to leave it open-ended because the dependency is mostly never breaking on majors and releasing majors quite often.
There was a problem hiding this comment.
Python 3.8 was dropped in 1.38.0. Set the versions to >=1.12.39,<1.38.0
There was a problem hiding this comment.
You need to use ranges on all dependencies. And probably adapt to Python versions supported, see other comment.
| @@ -0,0 +1,81 @@ | |||
| name: Publish Python 🐍 distribution 📦 to PyPI and TestPyPI | |||
There was a problem hiding this comment.
Why publishing on TestPyPi? This is not standard at all (we've never done that AFAIK) and doing this only on same release event than normal publishing seems useless. What was your point (please explain, you might have good reasons to do so that I totally miss)?
There was a problem hiding this comment.
I wanted to set up a test release that can be iterated upon without waiting for the final release. For example in cases like zimfarm where I'm waiting for this to be published, I had to use a local editable version
There was a problem hiding this comment.
But isn't workflow trigger the same?
| { name = "Kiwix", email = "dev@kiwix.org" }, | ||
| ] | ||
| keywords = ["kiwix", "zim", "offline", "aws", "s3"] | ||
| requires-python = ">=3.8,<3.14" |
There was a problem hiding this comment.
If it is useful, you can drop 3.8 ; if it is priceless, keep it
FYI I will also open a PR about adding support for 3.14 and dropping 3.9 which is going to be the next move (just so that you do not loose time on 3.9, we can drop it as well if it is too painful.
There was a problem hiding this comment.
If we drop support for 3.8, would it still be considered a minor upgrade? I was already using the pipe | form of annotations and it failed to run because it's not supported. That's why I decided to switch to Optional and Union. Do you still think I should drop the older language versions? I mean, I don't know if there are any other services using the library.
There was a problem hiding this comment.
Yes, we allow dropping Python version on minors (not patch)
And yes we don't mind about dropping unmaintained Python version. If code using this lib needs a new version, they have to upgrade to a maintained Python version. This is not that a big deal usually. And we don't want to waste our effort on maintaining for too many versions.
For the python-scraperlib we even took the decision to support only one single Python version since we are mostly the only users of this lib, and anyway, you want new features, you have to upgrade, otherwise you can stay with older versions with less features ... that's acceptable.
For python-libzim and this python-storagelib, we continue to support all maintained versions. But not more, it is just too much work, ecosystem move "fast".
|
Upgraded to minimum python 3.10 with support for 3.14 |
benoit74
left a comment
There was a problem hiding this comment.
We are getting closer, two minor modifications left and we are probably good to go
| ### Added | ||
|
|
||
| - configure resources with user-specified timeouts | ||
| - Requiring python3.10+ |
There was a problem hiding this comment.
Clearly mentioning range of supported Python versions (3.10 to 3.14) is going to make it easier for release note reader
| @@ -0,0 +1 @@ | |||
| __version__ = "0.9.1" | |||
There was a problem hiding this comment.
Update it to 0.10.0 since we are modifying support Python versions, we need a minor.
And for the time being, it should say 0.10.0-dev0 to clearly indicate anyone using it from source that it is a dev version, not the released 0.10.0 (helps avoiding lot's of confusion). We will update it to 0.10.0 when releasing.
benoit74
left a comment
There was a problem hiding this comment.
LGTM, thank you very much!
require minimum python version as 3.10
a8e89c8 to
a8cf990
Compare
Rationale
The PR migrates the repo to use the bootstrap pattern for python projects.
Changes
pyproject.hatchto specify dependenciesThis closes #15