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

Further typing tightening/modernization #129

Merged
merged 13 commits into from
Jan 12, 2024
Merged

Further typing tightening/modernization #129

merged 13 commits into from
Jan 12, 2024

Conversation

athornton
Copy link
Collaborator

Consolidating everything into pyproject.toml and making checks stricter.

Not much functional difference, but I'll explain what's going on:

  1. When we move to Ruff it's going to complain about Optional[type] since it prefers type|None, and I'm getting ahead of that.
  2. to do dynamic versioning requires git in the runtime container, so I added it.
  3. moved requirements to a subdirectory to reduce visual clutter for me.
  4. I don't know if the old PreAuthorizingTransferAdapter would have behaved correctly if you'd instantiated multiple subclasses, but there was a lot of shadowing going on at the very least with the handler being a class variable, so I made it also an instance variable and called super().init() from its subclasses. Because it inherits from ABC I probably don't need the @AbstractMethod decorator I have; the point is PreAuthorizingTransferAdapter should never be instantiated on its own, only as a superclass of the transfer adapter you are actually using.
  5. Scope was initializing its fields as both class and instance variables; removed the class variables.
  6. Removed release target from Makefile; for now I can build and push new containers manually, and eventually we will manage that from GitHub, and PyPi and Docker builds will be and stay in sync, release versions driven from Git tags.

@rufuspollock
Copy link
Member

LGTM.

@rufuspollock rufuspollock merged commit e2dad47 into datopian:main Jan 12, 2024
@athornton athornton deleted the tickets/DM-42231 branch January 12, 2024 20:48
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