-
Notifications
You must be signed in to change notification settings - Fork 21
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
Add a Python 3.9 container (jsc#PED-10823) #1757
base: main
Are you sure you want to change the base?
Conversation
Created a staging project on OBS for 6: home:defolos:BCI:Staging:SLE-15-SP6:6-1757 Build ResultsRepository
Repository
Repository
Repository
Repository
Repository
Repository
Repository
Build succeeded ✅ To run BCI-tests against this PR, use the following command: OS_VERSION=15.6 TARGET=custom BASEURL=registry.opensuse.org/home/defolos/bci/staging/sle-15-sp6/6-1757/ tox -- -n auto The following images can be pulled from the staging project:
|
b11d643
to
c14ac49
Compare
c14ac49
to
d34b917
Compare
d34b917
to
d5aa6a1
Compare
d5aa6a1
to
1aaf2d6
Compare
1aaf2d6
to
05ca7b2
Compare
94cb4cc
to
d2e373b
Compare
d2e373b
to
5e1c0b3
Compare
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.
This is fine in principle. I've added a few suggestions how to improve the Registry classes and I'm not entirely happy with the amount of pass through methods. But it's a good start in the right direction, thank you!
src/bci_build/registry.py
Outdated
def get_registry(container) -> RegistryABC: | ||
"""Return the appropriate registry for the container.""" | ||
if container.os_version.is_tumbleweed: | ||
return openSUSERegistry() | ||
return SUSERegistry() |
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.
Or allow to specify whether this is an appcol image and pass the os_version
as a parameter instead of the container, that would allow to remove the duplication of None if os_version.is_tumbleweed else ApplicationCollectionRegistry()
def get_registry(container) -> RegistryABC: | |
"""Return the appropriate registry for the container.""" | |
if container.os_version.is_tumbleweed: | |
return openSUSERegistry() | |
return SUSERegistry() | |
def get_registry(os_version: OsVersion, is_app_col: bool = False) -> RegistryABC: | |
"""Return the appropriate registry for the container.""" | |
if os_version.is_tumbleweed: | |
return openSUSERegistry() | |
return ApplicationCollectionRegistry() if is_app_col else SUSERegistry() |
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.
this was intentionally the way it is to reduce refactoring effort. using os_version means there are recursive imports
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.
splitted into a new toplevel module.
3c6dbf8
to
ebc5c8a
Compare
This removes a bit of if() condition wars in the BaseContainerImage and delegates it into its own class hierarchy, which allows to build a python container for appcol without having to do more subclasses.
This is required to avoid cyclic imports between bci_build.package and bci_build.registry.
rename get_registry to publish_registry and add a kwargs to set that wether to prefer AppCollection.
For AppCol..