← Back to issue list

lxd: split container classes into different files

View original Github issue

Metadata

Project
snapcraft
Number
#1627
Type
pull request
State
merged
Author
kalikiana
Labels
Created
Updated
Closed

Current evaluation

Merged refactoring splitting LXD container classes from lxd.py into separate modules. Improves organization, moves Cleanbuilder methods, de-duplicates env vars, and fixes mocking imports. No functional changes. Approved by two reviewers and passed CI.

Suggested action:

No scores available.

Issue body

- [x] Have you followed the [guidelines for contributing](https://github.com/snapcore/snapcraft/blob/master/CONTRIBUTING.md)? - [x] Have you signed the [CLA](http://www.ubuntu.com/legal/contributors/)? - [x] If this is a bugfix. Have you checked that there is a bug report open for the issue you are trying to fix on [bug reports](https://bugs.launchpad.net/snapcraft)? - [x] If this is a new feature. Have you discussed the design on the [forum](https://forum.snapcraft.io)? - [x] Have you successfully run `./runtests.sh static`? - [x] Have you successfully run `./runtests.sh unit`? ----- As lxd.py has been growing continously I'm splitting it up into one file per class in its own module. That's the Containerbuild base class, Cleanbuilder and Project. I'm using the opportunity to move the Cleanbuilder-specific methods _push_file, _pull_file, _setup_project and _finish out of the Containerbuild class, where before the separation was poor. I'm also de-duplicating calls to set environment variables. Also as a side effect of this split I had to stop importing subprocess functions and sleep by name because unittest.mock couldn't override them for the whole module and patching all the individual files would have been complex and error prone. There's no change in functionality here.

Evaluation history

Date Model Scores Action Summary
qwen/qwen3.6-35b-a3b Merged refactoring splitting LXD container classes from lxd.py into separate modules. Improves organization, moves Cleanbuilder methods, de-duplicates env vars, and fixes mocking imports. No functional changes. Approved by two reviewers and passed CI.
qwen3.6-35b-a3b-mtp-q6 Merged a refactoring splitting lxd.py into separate modules for Containerbuild, Cleanbuilder, and Project classes. Improves organization, moves methods appropriately, de-duplicates env variable handling, and fixes unittest.mock compatibility. No functional changes.
qwen3.6-35b-a3b-mtp-q6 Merged a refactor splitting lxd.py into separate modules for Containerbuild, Cleanbuilder, and Project. Improves separation, de-duplicates environment setup, and fixes unittest.mock patching via import changes. No functional changes.

Update history

No update history recorded yet.

Related issues

Issue Project State Summary Similarity
#1230 lxd: refactor Cleanbuilder into Containerbuild and add Project. snapcraft merged Merged. Refactored Cleanbuilder into a Containerbuild base class and introduced a Project class to manage persistent LXD containers for snap builds. Approved after code review and test fixes.
82%
#1762 lxd: delete container only if parts is empty snapcraft merged Merged fix resolving bug 1734145 by updating LXD container cleanup logic to only delete containers when parts are empty. Approved by one reviewer, passed CI checks, and modified two files.
77%
#2095 lxd: proper error classes for container errors snapcraft merged Merged after approval. Refactored LXD container errors into dedicated classes, relocated them to the LXD module, and fixed architecture retrieval error handling. Added corresponding unit tests. Resolves Launchpad bugs #1734233 and #1734235.
76%
#1644 lxd: fix the push in container builds snapcraft merged Merged pull request fixing LXD container push builds. Approved by one reviewer, the change refactored test cases for better coverage and updated build logic. Review comments highlighted the need for Travis integration and broader testing beyond unit tests.
74%
#1434 lxd: clean with no parts should only delete snapcraft merged Merged. Updates snapcraft LXD handling so running clean without arguments only deletes container resources instead of executing commands inside them. Approved by one reviewer and merged.
74%
#1403 lxd: Only remove container if one exists snapcraft merged Merged following two reviewer approvals and passing CI. The update resolves duplicate exceptions during failed LXD container creation by adding a conditional check before attempting container removal.
74%
#817 lxd: use built-in image streams. snapcraft merged Merged. Replaced custom remote setup for cleanbuilds with LXD built-in image streams, streamlining configuration. Approved by one reviewer with zero unresolved comments. The two-file change (+4/-13 lines) resolves LP #1625787.
72%
#1978 ci: switch to stable lxd and unconfined containers snapcraft merged Merged CI configuration update switching to stable LXD and unconfined containers. Approved by two reviewers with a two-file, five-line change.
72%
#3112 spread: remove dead code for lxd setup and add debug prints snapcraft merged Merged removal of redundant elif dead code in LXD setup logic and addition of debug prints for LXD snap install failures. Approved by one reviewer, passed CI checks, and integrated.
72%
#1364 lxd: Inject snapcraft and core snaps into the container snapcraft merged Merged. Injects the host's snapcraft and core snaps into LXD containers to ensure build version consistency. Approved by one reviewer, modifying nine files with 360 additions and 46 deletions.
71%