Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Future code base components? (python 3.12 / pyproject.toml vs setup.py / poetry vs pip-pipenv-twine-... / docker improvements) #181

Open
wants to merge 50 commits into
base: master
Choose a base branch
from

Conversation

stynoo
Copy link
Contributor

@stynoo stynoo commented Nov 13, 2024

When working on the last PR I came across the statements that "The recommended way of using Setuptools has shifted in the direction of using the pyproject.toml in favour of setup.py and setup.cfg."
Combining this with "pkg_resources is deprecated" in python 3.12 and the docker wishlist in #133 I realized that we probably will have to introduce some breaking changes in the future.
Hence introducing this PR, primarily as a proposition of a possible way forwards. This introduces the following changes:

  1. move away from setuptools/setup.py/requirements.txt and start using poetry as a package/venv builder-manager based on pyproject.toml
  2. base everything on python3.12 and eliminate all deprecated dependencies and warnings
  3. build a small lightweight docker image based on some best practices such as layer optimization & running the app as a non-priviledged user
  4. update github actions to make use of this new way of working

Again, this will introduce breaking changes, this PR is incomplete but I wanted to share this early to start a discussion/collaboration. If this is not the correct way forwards or not what you envisioned, just let me know.

todo:

  • update/cleanup README, especially the parts that involve crontab & the use of /root directory in the container
  • test/update release mechanism, I am not sure how versioning is handled nowadays
  • test on previous python versions (is 3.12 a hard cutoff?)

Need help with Kubernetes/K8s files in /contrib

Copy link
Collaborator

@longstone longstone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've taken a quick look and appreciate the changes overall. However, I have a few concerns about releasing with GitHub Actions that could use some improvement.

In summary:

  • We should eliminate do_release.sh.
  • The release version should be taken directly from Git tags, as it currently works this way.
  • Running Docker as a non-root user is a significant advantage.

Unfortunately, I’ll have limited availability over the next two weeks.

contrib/do_release.sh Outdated Show resolved Hide resolved
Dockerfile Show resolved Hide resolved
.github/workflows/build-publish.yml Outdated Show resolved Hide resolved
.github/workflows/build-publish.yml Show resolved Hide resolved
@stynoo
Copy link
Contributor Author

stynoo commented Nov 15, 2024

Thx for taking the time already, I don't think we need to rush this, nothing is on fire atm.
I'll try to dive deeper into the questions during the next weeks but my availability is also scarce.
I may need some help to optimize the release/version process.

@stynoo
Copy link
Contributor Author

stynoo commented Dec 7, 2024

It's at a point now where I think the building and publishing of both package & container image should work (triggered from the Github create-new-release function). I am not sure if you would be able to test that separately.

The non-root user / python 3.12 requirement still introduces some breaking changes to end users setups so extra steps need to be taken to tackle this.

@stynoo
Copy link
Contributor Author

stynoo commented Dec 18, 2024

Right we are getting somewhere, lot's of changes in the readme and covering all items in #133 . I am moving my own install over to containers built from https://github.com/stynoo/withings-sync to test the changes.
My main test env will be PI5 / ARM / dietpi / docker-compose / supercronic option.

Please have a look, especially at the way the readme is progressing and let me know what you think (at your convenience ofc).

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.

2 participants