-
Notifications
You must be signed in to change notification settings - Fork 76
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
Missing importlib_metadata requirement #1176
Comments
Consolidating information from another issue and Slack:
@maukoquiroga @benjello you respectively authored and approved the changeset that introduced the regression, can you fix this? |
We also have to understand, as a postmortem, what is missing in the test suite that enabled several versions to be published with such a major problem. |
@MattiSG I will push a fix, yet it is IMHO incorrect to label it as a regression, as it is a problem caused by an external change in dependencies (it gets frustrating by the day, I know). In fact, I think just reverting would still make flake8 to fail because of incompatible versions with twine (keyring) as @eraviart reported. By the way thanks @eraviart for #1177 It's hard to provide an analytical postmortem at this point, as most of what is happening seems related to poor dependency management and very old code (pkg_ressources use had to be refactored). Concerning #1168, it includes one possible fix for this, but changes were requested because of the publishing job failing (thanks for testing) so it invariably requires a new review. Appart from the manual workflow dispatch configuration, the scope of that PR has actually been reduced. The reason I chose to split the workflows was to make it easier to spot bugs like the publishing one that you reported. I understand the frustration of splitting the files, however I wouldn't feel comfortable integrating the version as it was first reviewed. Yet, extracting the changes concerned with the reported error is easy 😃 |
I think the best solution is to manage build, test and deploy dependencies in isolation. #1161 goes in that direction. However, it seems unlikely to avoid this 100%, but that would make a great difference IMHO. |
After taking a closer look, #1165 did actually broke builds under some configurations, in particular in python versions 3.8+, the culprit being a failure to declare a dependency that was being overridden by others in a least two scenarios:
The direct postmortem I can draw now, with a better distance from the issue:
|
Since #1165,
openfisca_core/taxbenefitsystems/tax_benefit_system.py
importsimportlib_metadata
, but that package is missing from requirements insetup.py
.So every use of a TaxBenefitSystem fails.
The text was updated successfully, but these errors were encountered: