Prefer the 'apt_pkg' module over 'apt' (SC-1516) #2744
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
|
Jira: SC-1516 GitHub Issues: No GitHub issues are fixed by this PR. (No commits have Fixes: #### references) Launchpad Bugs: No Launchpad bugs are fixed by this PR. (No commits have LP: #### references) Documentation: The changes in this PR do not require documentation changes. 👍 this comment to confirm that this is correct. |
renanrodrigo
force-pushed
the
prefer_apt_pkg
branch
4 times, most recently
from
c77b2b7
to
a6448c9
Compare
renanrodrigo
changed the title
renanrodrigo
requested review from
lucasmoura,
orndorffgrant and
julian-klode
renanrodrigo
force-pushed
the
prefer_apt_pkg
branch
from
a95a39d
to
594cfe4
Compare
lucasmoura
reviewed
Sep 22, 2023
|
|
renanrodrigo
force-pushed
the
prefer_apt_pkg
branch
5 times, most recently
from
25ab041
to
e538dc7
Compare
|
CI Failures unrelated - a cloud timeout and #2756 |
lucasmoura
reviewed
Sep 26, 2023
renanrodrigo
force-pushed
the
prefer_apt_pkg
branch
from
e538dc7
to
2b255ed
Compare
|
I'll try to have a look tomorrow |
orndorffgrant
approved these changes
Sep 28, 2023
renanrodrigo
force-pushed
the
prefer_apt_pkg
branch
from
8147015
to
7274b6d
Compare
5 tasks
Signed-off-by: Renan Rodrigo <[email protected]>
Signed-off-by: Renan Rodrigo <[email protected]>
Remove the global mock and install python-apt from source. This way we can have access to the 'apt' and 'apt_pkg' modules during test execution. This revealed a fixture we missed on the apt update functionality and some mypy complains we could have addressed, and simplified a couple tests. Signed-off-by: Renan Rodrigo <[email protected]>
Signed-off-by: Renan Rodrigo <[email protected]>
Signed-off-by: Renan Rodrigo <[email protected]>
As we are not creating the cache using the apt module anymore, a new helper function is needed to ensure the structure is there before we set up the sources.list files Signed-off-by: Renan Rodrigo <[email protected]>
Signed-off-by: Renan Rodrigo <[email protected]>
Signed-off-by: Renan Rodrigo <[email protected]>
Also fix a unit test that was passing based on luck Signed-off-by: Renan Rodrigo <[email protected]>
It is more resilient now, but we can log success and failure using the AcquireProgress object Signed-off-by: Renan Rodrigo <[email protected]>
Instead of performing the check at the module level, we decorate the functions that use the module without instantiating a Cache object (in the Cache case, we initialize apt_pkg separately) Signed-off-by: Renan Rodrigo <[email protected]>
renanrodrigo
force-pushed
the
prefer_apt_pkg
branch
from
7274b6d
to
2498ff2
Compare
|
@julian-klode we need to merge for the release, but any further review or comment is welcome - we can address those in next iteration |
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.
Why is this needed?
This PR solves all of our problems because we are trying to standardize the use of
python3-aptthrough the codebase. After some discussions with @julian-klode in the past, we have decided to stick toapt_pkginstead ofaptwhen possible, so we would:aptis breaking our isolation of configuration, given that we are responsible for the esm-apt instanceapt_pkgcapabilities.Test Steps
This is just a medium-size refactor; no new features introduced, so CI should pass without major changes (although adjustments in unit tests were of course needed because we still test some implementation details and isolation is not 100%)
It is possible to verify performance gains, maybe where it is more impactful: run
pro security-statuson an updated unattached bionic (any release, really) machine both with and without this patch and compare the differences.Checklist
Does this PR require extra reviews?