-
Notifications
You must be signed in to change notification settings - Fork 5
Controller: format agnostic package interface #41
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
base: master
Are you sure you want to change the base?
Conversation
Introduce a generic interface to load, check, build, test and publish packages, with abstract classes Package and ActionableArchPackage. All RPM specifics are now handled by children classes PackageRPM and ActionableArchPackageRPM classes. The Controller build, test and validate actions are updated to use the new generic interface. This is one step to make rift able to support other package formats, in addition to RPM. ProjectPackages class is also introduced. This is a factory to retrieve project's packages as concrete package objects with the corresponding format. It replaces Package.list() generator. ProjectArchRepositories do not support extra repositories anymore. This was used to hold staging repository. The staging repository is now directly provided to build, test and publish methods. The Package specfile attribute is renamed buildfile to make it more generic as well. Some modules still expect actual RPM packages. They now instanciate concrete PackageRPM objects. This implies many small changes throughout the code base and the tests.
qa-cea
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not really getting the difference between the Package and the ActionableArchPackage classes, do we really need two abstract classes ?
lib/rift/package/_base.py
Outdated
| self._modules = modules | ||
| self.name = name | ||
| # check package format | ||
| if _format != 'rpm': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: maybe having of supported format somewhere in the code would be easier to maintain ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I initially planned to introduce this later but you're right, it is probably a good idea to introduce it now 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done with 216fecc.
lib/rift/package/rpm.py
Outdated
| def __init__(self, name, config, staff, modules): | ||
| super().__init__(name, config, staff, modules, 'rpm', f"{name}.spec") | ||
|
|
||
| # Attribytes assigned in load() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed with 1bdddbf.
A An This model has been chosen because IMO it is well suited to Controller build and validation logic. It notably avoids providing the arch argument to all methods build/test/publish/clean methods. YMMV though. If you prefer a model with just a |
|
I understand better now, thank you. Let's wait for Yoann input on this. |
Make Package and ActionableArchPackage classes real abtract classes with abc module. Also add tests to cover more base classes code.
Introduce this set as single source of thruth for all package formats supported by Rift.
216fecc to
50cdcbc
Compare
Introduce a generic interface to load, check, build, test and publish packages, with abstract classes Package and ActionableArchPackage. All RPM specifics are now handled by children classes PackageRPM and ActionableArchPackageRPM classes. The Controller build, test and validate actions are updated to use the new generic interface. This is one step to make rift able to support other package formats, in addition to RPM.
ProjectPackages class is also introduced. This is a factory to retrieve project's packages as concrete package objects with the corresponding format. It replaces Package.list() generator.
ProjectArchRepositories do not support extra repositories anymore. This was used to hold staging repository. The staging repository is now directly provided to build, test and publish methods.
The Package specfile attribute is renamed buildfile to make it more generic as well. Some modules still expect actual RPM packages. They now instanciate concrete PackageRPM objects. This implies many small changes throughout the code base and the tests.