-
Notifications
You must be signed in to change notification settings - Fork 97
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
stagingapi: carry over build state during supersede. #897
Conversation
4d9e521
to
f8cc20e
Compare
so anyone confirmed this working? |
We could either risk it, create a manual scenario, or wait for me to have time to rework testing setup so this is not a pain to test. |
delete_package(self.apiurl, subprj, package, force=True, msg=msg) | ||
|
||
for sub_prj, sub_pkg in self.get_sub_packages(package): | ||
sub_prj = self.map_ring_package_to_subject(project, sub_pkg) | ||
if self._supersede: |
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.
Actually this should not needed, ring package should always be enabled to build and it's sub-package should do too, carry over build state is only apply to new package
request.
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.
you mean 'new in ring'? as we might have an existing package that is newly promoted to a ring, paired with a submission introducing new fixes needed too
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.
Whatever it's new package submission to Factory and will be promoted to a ring or an existing package update and will be promoted to a ring, Rings should not have changed until the submission merged to Factory, thus there is none
from get_sub_packages
.
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.
My thought at this point is two-fold:
- Lets prioritize Bring up real OBS against which tests may be run #1002 (real OBS to test against) so we have some way to walk through all the possible workflows and verify since this is some fairly scary code.
- It may make sense for me to revisit my prototype
ring
command (ring: create command for making and recording ring changes and process during accept #848) and finish it as that would provide supplemental information that would make it crystal clear which packages should be enabled rather than the somewhat wishy-washy alternatives (looking at previous build settings).
I'd like to give it a try in real however staging-bot always win the game :-) |
hey @jberry-suse , I tested your code, there is a bug about join(), in this change there is two arguments given to join() hence it fails to work, I think it should mean a tuple of strings ie. |
Allows for packages being added to rings to remain enabled after being superseded.
f8cc20e
to
00dee0d
Compare
Thanks for testing. I made the following changes. diff --git a/osclib/stagingapi.py b/osclib/stagingapi.py
index 7530d4d..9c2407c 100644
--- a/osclib/stagingapi.py
+++ b/osclib/stagingapi.py
@@ -782,7 +782,7 @@ class StagingAPI(object):
meta = ET.fromstring(''.join(meta))
disabled = len(meta.xpath('build/disable[not(@*)]')) > 0
if store:
- self._package_disabled['/'.join(project, package)] = disabled
+ self._package_disabled['/'.join([project, package])] = disabled
return disabled
def create_package_container(self, project, package, disable_build=False):
@@ -1119,7 +1119,7 @@ class StagingAPI(object):
project = self.map_ring_package_to_subject(project, tar_pkg)
if self._supersede:
- disable_build = self._package_disabled.get('/'.join(project, tar_pkg), disable_build)
+ disable_build = self._package_disabled.get('/'.join([project, tar_pkg]), disable_build)
self.create_package_container(project, tar_pkg,
disable_build=disable_build)
@@ -1147,7 +1147,7 @@ class StagingAPI(object):
if sub_prj == project: # skip inner-project links
continue
if self._supersede:
- disable_build = self._package_disabled.get('/'.join(sub_prj, sub_pkg), False)
+ disable_build = self._package_disabled.get('/'.join([sub_prj, sub_pkg]), False)
self.create_package_container(sub_prj, sub_pkg, disable_build=disable_build)
root = ET.Element('link', package=tar_pkg, project=project) |
Allows for packages being added to rings to remain enabled after being superseded.
Likely need to test on a local OBS, build out a supersede test case (not trivial), or cross-fingers.
DO NOT ACCEPT UNTIL SOMEONE HAS HAD A CHANCE TO SEE THIS WORK.
Fixes #896.
Presumably, it is desirable to keep sub package states which is what I did.