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

Upgrade tooling #126

Merged
merged 8 commits into from
Jan 21, 2025
Merged

Upgrade tooling #126

merged 8 commits into from
Jan 21, 2025

Conversation

Batalex
Copy link
Contributor

@Batalex Batalex commented Jan 10, 2025

Changes

  • Align our tooling with our other products
  • Drop support for Python 3.8 and 3.9. Our default runtime is Jammy, which comes with 3.10
    • upgraded type hints for a more modern code base
  • Fix compatibility with Python 3.11+
  • The previous flake8 configuration was not properly enforced for some reason. Migrating to ruff made me fix a bunch of violations (which means adding low value docstrings, to be honest)

We have some unused or outdated stuff that we could remove:

  • utils.py
    • union(): we use it for PropertyFile objects which are only one level data structure AFAICT. The | operator is sufficient
    • _check(): used in one place in filter_none, trivial to inline this
    • mkdir()
    • create_dir_if_not_exists()
  • exceptions.py
    • K8sClusterNotReachable
  • services.py
    • InMemoryAccountRegistry is only used in unit tests, maybe it could go there? We would not have to distribute it in the package

@Batalex Batalex self-assigned this Jan 10, 2025
@Batalex Batalex force-pushed the upgrade-tooling branch 2 times, most recently from 97181a9 to 7314218 Compare January 13, 2025 09:57
@Batalex Batalex marked this pull request as ready for review January 13, 2025 10:59
Copy link
Contributor

@deusebio deusebio left a comment

Choose a reason for hiding this comment

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

Looks great @Batalex ! Thank you for this chore work!!!

Copy link
Member

@theoctober19th theoctober19th left a comment

Choose a reason for hiding this comment

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

Great work publishing spark8t in PyPi! The PR looks superb!

@Batalex
Copy link
Contributor Author

Batalex commented Jan 15, 2025

@deusebio, @theoctober19th thanks for the reviews! What are your thoughts on removing the unused elements I listed in the description?

@theoctober19th
Copy link
Member

@Batalex I'm fine with updating / refactoring places where the code is not used / relevant anymore, (eg, unused exceptions, functions which can be replaced with simple inbuilt structures, etc.). One thing I'm not quite sure about is the purpose of the InMemoryAccountRegistry. I'd like to hear @welpaolo and @deusebio's views on the original motive behind this registry before I comment anything on that particular piece of code.

@Batalex Batalex linked an issue Jan 16, 2025 that may be closed by this pull request
@Batalex Batalex merged commit 450e031 into main Jan 21, 2025
4 checks passed
@Batalex Batalex deleted the upgrade-tooling branch January 21, 2025 11:29
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.

Breaking changes in enumerators in Python 3.11
3 participants