Remove globals in policy module #466
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lkollar
force-pushed
the
policy-refactor
branch
2 times, most recently
from
2d4b03b
to
13d891f
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #466 +/- ##
==========================================
+ Coverage 92.04% 92.05% +0.01%
==========================================
Files 22 20 -2
Lines 1244 1246 +2
Branches 296 298 +2
==========================================
+ Hits 1145 1147 +2
Misses 56 56
Partials 43 43 ☔ View full report in Codecov by Sentry. |
mayeut
reviewed
Dec 9, 2023
mayeut
reviewed
Dec 9, 2023
mayeut
reviewed
Dec 9, 2023
mayeut
reviewed
Dec 9, 2023
The policy module creates several globals on the module level, which makes it very difficult to test code which interacts with the policies. This patch creates a `WheelPolicies` class which encapsulates the policy processing code and exposes functions to query the parsed policies. Unfortunately, a lot of functions directly operate on policies and these all need a new argument to take the new policy object. This change should make testing substantially easier as policies can be easily created on the fly and passed down to functions which require these.
These had to be imported at the bottom to avoid a circular import. Now that we got rid of the globals in the policy module, we can move these into the right place.
The `external_versioned_symbols` and `versioned_symbols_policy` functions used to live in two separate files, importing code from the `__init__.py` file in the same module. As this file itself imports both of these symbols, this creates a cirtular import. We can easily break this by moving the two functions into the same `__init__.py` file.
lkollar
force-pushed
the
policy-refactor
branch
from
13d891f
to
8c18c7c
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I started working on this when I created #460 and ran into the problem with the globals in the
policymodule once again. These globals make it very difficult to properly test simple changes as the module-level globals get initialised on the first import which usually happens before any mocking can be done.This is a large PR, but most of the changes are just moving code around.