= Depcheck workflow =
After discussing some issues with Kamil, we decided, that it would be for the best, if we wrote down the intended Depcheck results usage & workflow.
The problem is tightly connected to #306. The problem described in #306 means this:
== Premise ==
"Updates can be changed in Bodhi." - the evidence is clear https://admin.fedoraproject.org/updates/control-center-3.0.0-1.fc15,gnome-settings-daemon-3.0.0.1-1.fc15, if you look at the bodhi comment from 2011-04-06 15:05:38 "hadess has edited this update..."
== Implications ==
=== Timeline ===
1) All updates in -pending passed depcheck, and have "PASSED" comment. 2) Update XYZ is changed by the developer
=== Consequences ===
Because one of the "already passed" updates is changed, we should discard the whole "passed" subset, because we can not be sure, that some errors were not introduced by the change.
== Level 1 Solution ==
#306 describes the 'simple' solution. When we search for the 'accepted' updates, we look for all the updates, which do have "PASSED depcheck" comment in bodhi.
We want to detect, whether the update was changed after the "PASSED depcheck" comment, and if so, then remove it from accepted set, and basicaly retest it.
== Level 2 Solution ==
Level 1 solution does not really solve the problem, as described, even though it is certainly needed to detect the 'passed but changed', and will sort out some of the problems.
The problem with the fact, that change in one update can possibly break some 'previously passed' dependency problems is still there.
Because it would be incredibly hard to detect just the updates influenced by changes in the respective update, we decided to define a workflow, which bypasses the overall need for this.
=== RelEng workflow ===
At the moment, rel-eng is pushing updates manualy few times a week [citation needed, this is informal information which has not been audited].
The thing is, that we can provide the _at the moment 'pushable'_ subset of packages. When rel-eng will want to push from -pending to {stable, updates, ...}, they'll just 'list' the pushable subset (yes, resultsdb will help ;-)).
"The tool" will detect if there were any changes in the pushable subset, and "the tool" will inform rel-eng that they either need to wait for the results of the next depcheck run, or (if we're able to do it) request new depcheck run.
On 04/14/2011 04:02 AM, Josef Skladanka wrote:
= Depcheck workflow =
After discussing some issues with Kamil, we decided, that it would be for the best, if we wrote down the intended Depcheck results usage & workflow.
The problem is tightly connected to #306. The problem described in #306 means this:
== Premise ==
"Updates can be changed in Bodhi." - the evidence is clear https://admin.fedoraproject.org/updates/control-center-3.0.0-1.fc15,gnome-settings-daemon-3.0.0.1-1.fc15, if you look at the bodhi comment from 2011-04-06 15:05:38 "hadess has edited this update..."
== Implications ==
=== Timeline ===
- All updates in -pending passed depcheck, and have "PASSED"
comment. 2) Update XYZ is changed by the developer
=== Consequences ===
Because one of the "already passed" updates is changed, we should discard the whole "passed" subset, because we can not be sure, that some errors were not introduced by the change.
== Level 1 Solution ==
#306 describes the 'simple' solution. When we search for the 'accepted' updates, we look for all the updates, which do have "PASSED depcheck" comment in bodhi.
We want to detect, whether the update was changed after the "PASSED depcheck" comment, and if so, then remove it from accepted set, and basicaly retest it.
I suppose that the easiest solution would be to parse the comments on an update as a whole (instead of one-by-one) and sort by date. If there was an update comment after the 'PASSED' comment, is_bodhi_commented() returns false.
I wonder how hard it would be to change the way depcheck and bodhi work a little bit.
Instead of just scraping comments, another option would be to have a text field in bodhi that held the list of passing tests. When a test passes, it adds its name to the field. When an update is changed, that field is reset and the tests are re-run.
It might make more sense to wait for resultsdb on this one, I'm not sure. Either one but might be more accurate and faster than trying to interpret comments.
Do we know how often this is happening?
== Level 2 Solution ==
Level 1 solution does not really solve the problem, as described, even though it is certainly needed to detect the 'passed but changed', and will sort out some of the problems.
The problem with the fact, that change in one update can possibly break some 'previously passed' dependency problems is still there.
Because it would be incredibly hard to detect just the updates influenced by changes in the respective update, we decided to define a workflow, which bypasses the overall need for this.
=== RelEng workflow ===
At the moment, rel-eng is pushing updates manualy few times a week [citation needed, this is informal information which has not been audited].
The thing is, that we can provide the _at the moment 'pushable'_ subset of packages. When rel-eng will want to push from -pending to {stable, updates, ...}, they'll just 'list' the pushable subset (yes, resultsdb will help ;-)).
"The tool" will detect if there were any changes in the pushable subset, and "the tool" will inform rel-eng that they either need to wait for the results of the next depcheck run, or (if we're able to do it) request new depcheck run.
Why not just make a tool that re-reruns all "*-pending" updates through a battery of tests (depcheck and upgradepath ATM), ignoring any "PASSED" results and not posting any new comments to bodhi unless there was a change for an update? That way we don't have to worry about detecting if there were changes or not and we can be more confidant of the to-be-pushed set as a whole (which is what our end goal is, IIRC)
One easy way to do this for the short term is to have some sort of CLI or webpage that does a "pre-push" test suite and runs upgradepath and depcheck (more later, maybe) on everything that is about to be pushed, regardless of what tags and comments already exist.
This could be done locally but might be more productive to push a job to autotest and have it run on the test machines so that whoever is doing the pushing doesn't have to download all the packages.
Tim
I wonder how hard it would be to change the way depcheck and bodhi work a little bit.
Instead of just scraping comments, another option would be to have a text field in bodhi that held the list of passing tests. When a test passes, it adds its name to the field. When an update is changed, that field is reset and the tests are re-run.
That would be a nice solution if we didn't have ResultsDB in our plan.
It might make more sense to wait for resultsdb on this one, I'm not sure. Either one but might be more accurate and faster than trying to interpret comments.
Sending bodhi comments was just a quick solution for letting maintainers know. Instead of spending time on further temporary hacks (like hacking into Bodhi something that won't be needed soon) I see as a better solution to spend the time on the proper solution - ResultDB. It's not hard, it just needs focus.
Do we know how often this is happening?
Sometimes. Maintainers change their updates every now and then. Level 1 solution (re-test just the changed update) is extremely easy to implement and eliminates a lot of problems. So it's reasonable to spend a few hours and have it done.
=== RelEng workflow ===
At the moment, rel-eng is pushing updates manualy few times a week [citation needed, this is informal information which has not been audited].
The thing is, that we can provide the _at the moment 'pushable'_ subset of packages. When rel-eng will want to push from -pending to {stable, updates, ...}, they'll just 'list' the pushable subset (yes, resultsdb will help ;-)).
"The tool" will detect if there were any changes in the pushable subset, and "the tool" will inform rel-eng that they either need to wait for the results of the next depcheck run, or (if we're able to do it) request new depcheck run.
Why not just make a tool that re-reruns all "*-pending" updates through a battery of tests (depcheck and upgradepath ATM), ignoring any "PASSED" results and not posting any new comments to bodhi unless there was a change for an update? That way we don't have to worry about detecting if there were changes or not and we can be more confidant of the to-be-pushed set as a whole (which is what our end goal is, IIRC)
That's exactly what upgradepath does now. Depcheck is little different, it uses the concept of 'accepted updates'.
We can't remove the 'accepted updates' concept from depcheck, because that could cause a flood of emails going to maintainers' mailboxes through bodhi comments. If we don't use the concept of ever-growing accepted set of updates, it might happen that your update will be accepted at 1 PM, rejected at 2 PM (someone pushed conflicting update), accepted at 3 PM (someone removed the conflicting update), etc. Hence the Level 1 solution, which is not perfect, but doesn't spam maintainers.
After we have the concept of ResultDB and we abolish the concept of sending emails to maintainers for every single change in depcheck results, we can do the proper solution as described by Josef. That means emptying the 'accepted set' if some update from that set has been changed. It also means providing the latest never-outdated results directly to RelEng (or anyone else interested) on request, not continuously to package maintainers. If _in that moment of RelEng push_ some update is excluded because of failed dependencies, only after that we can send email to the maintainer. (Or some similar approach).
To sum it up, the really proper solution is not possible as long as we rely on sending emails to maintainers for every change. That's not viable, just temporary, because we could easily turn into spammers.
Comments intended for my clarification on the role of depcheck and differences between builds and updates.
On Fri, 2011-04-15 at 05:40 -0400, Kamil Paral wrote:
I wonder how hard it would be to change the way depcheck and bodhi work a little bit.
Instead of just scraping comments, another option would be to have a text field in bodhi that held the list of passing tests. When a test passes, it adds its name to the field. When an update is changed, that field is reset and the tests are re-run.
That would be a nice solution if we didn't have ResultsDB in our plan.
We might want to get in touch with lmacken, I think there are some rough plans for that type of karma feedback.
It might make more sense to wait for resultsdb on this one, I'm not sure. Either one but might be more accurate and faster than trying to interpret comments.
Sending bodhi comments was just a quick solution for letting maintainers know. Instead of spending time on further temporary hacks (like hacking into Bodhi something that won't be needed soon) I see as a better solution to spend the time on the proper solution - ResultDB. It's not hard, it just needs focus.
Do we know how often this is happening?
Sometimes. Maintainers change their updates every now and then. Level 1 solution (re-test just the changed update) is extremely easy to implement and eliminates a lot of problems. So it's reasonable to spend a few hours and have it done.
=== RelEng workflow ===
At the moment, rel-eng is pushing updates manualy few times a week [citation needed, this is informal information which has not been audited].
The thing is, that we can provide the _at the moment 'pushable'_ subset of packages. When rel-eng will want to push from -pending to {stable, updates, ...}, they'll just 'list' the pushable subset (yes, resultsdb will help ;-)).
"The tool" will detect if there were any changes in the pushable subset, and "the tool" will inform rel-eng that they either need to wait for the results of the next depcheck run, or (if we're able to do it) request new depcheck run.
Why not just make a tool that re-reruns all "*-pending" updates through a battery of tests (depcheck and upgradepath ATM), ignoring any "PASSED" results and not posting any new comments to bodhi unless there was a change for an update? That way we don't have to worry about detecting if there were changes or not and we can be more confidant of the to-be-pushed set as a whole (which is what our end goal is, IIRC)
That's exactly what upgradepath does now. Depcheck is little different, it uses the concept of 'accepted updates'.
We can't remove the 'accepted updates' concept from depcheck, because that could cause a flood of emails going to maintainers' mailboxes through bodhi comments. If we don't use the concept of ever-growing accepted set of updates, it might happen that your update will be accepted at 1 PM, rejected at 2 PM (someone pushed conflicting update), accepted at 3 PM (someone removed the conflicting update), etc. Hence the Level 1 solution, which is not perfect, but doesn't spam maintainers.
My understanding to the original design goal with depcheck was that once an update was tested and accepted ... it must not be revoked. The set of accepted updates must always be dep-free. If new builds are added to an update, those builds must represent a new set of -pending updates for testing. If another update lands in -pending, and causes problems with some previously accepted update ... the problem is with the new update, not with something already accepted.
Does this design goal still hold?
After we have the concept of ResultDB and we abolish the concept of sending emails to maintainers for every single change in depcheck results, we can do the proper solution as described by Josef. That means emptying the 'accepted set' if some update from that set has been changed.
When new builds are added to an existing update, the previous builds may have already moved on to updates-[testing] and will be automatically included in future depcheck runs. If they aren't in updates[-testing] yet, will bodhi remove the -pending tags from the previous builds, or will there be two versions of the same package still in -pending?
One point I get confused no here is thinking about updates and builds. With depcheck, is it safe to say we test builds, but provide feedback against updates? When new builds are added to an existing update ... they are tagged with -pending right? As long as that's true, they'll be treated as new builds, right? It's our update feedback procedure that needs to accommodate for an already tested, but modified, update?
The issue in question now is if those new builds introduce a dependency conflict? In that case, we should note the failure in the bodhi update since the results are against newer builds (and remove any previously supplied karma </future>)?
It also means providing the latest never-outdated results directly to RelEng (or anyone else interested) on request, not continuously to package maintainers. If _in that moment of RelEng push_ some update is excluded because of failed dependencies, only after that we can send email to the maintainer. (Or some similar approach).
To sum it up, the really proper solution is not possible as long as we rely on sending emails to maintainers for every change. That's not viable, just temporary, because we could easily turn into spammers.
Agreed.
Thanks, James
My understanding to the original design goal with depcheck was that once an update was tested and accepted ... it must not be revoked. The set of accepted updates must always be dep-free. If new builds are added to an update, those builds must represent a new set of -pending updates for testing. If another update lands in -pending, and causes problems with some previously accepted update ... the problem is with the new update, not with something already accepted.
Does this design goal still hold?
Yes, it does. Unless some update in the accepted set changes.
Even though the updates should not influence each other (all the related builds should be grouped together in a single update), it's naive to suppose it's always the case. If we presume that sometimes two different updates can influence each other (which is the main presumption of our whole depcheck testing in the first case, btw), then we have to clear the accepted set and start building it again when some already accepted update has been changed. Because it could have broken not only itself, but also some other already accepted updates.
That's the Level 2 (long-term) solution.
After we have the concept of ResultDB and we abolish the concept of sending emails to maintainers for every single change in depcheck results, we can do the proper solution as described by Josef. That means emptying the 'accepted set' if some update from that set has been changed.
When new builds are added to an existing update, the previous builds may have already moved on to updates-[testing] and will be automatically included in future depcheck runs. If they aren't in updates[-testing] yet, will bodhi remove the -pending tags from the previous builds, or will there be two versions of the same package still in -pending?
I suppose that if I have update "foo-1.1,bar-1.1" and then edit the update and replace "foo-1.1" for "foo-1.2", then "foo-1.1" should be untagged from updates-testing and "foo-1.2" should be tagged into updates-testing. Confirmation with lmacken needed. I believe this is exactly the kind of issues we've been hitting lately with Bodhi (untagging wasn't done right).
This only concerns updates-testing. Once it is in stable updates, you can't edit it.
One point I get confused no here is thinking about updates and builds. With depcheck, is it safe to say we test builds, but provide feedback against updates?
Correct, we say that update has passed <=> all its builds have passed.
When new builds are added to an existing update ... they are tagged with -pending right? As long as that's true, they'll be treated as new builds, right?
They will, once we fix https://fedorahosted.org/autoqa/ticket/306
Currently, we would suppose the new build was already tested, because its parent update already has our comments.
It's our update feedback procedure that needs to accommodate for an already tested, but modified, update?
Yes, we need to accomodate bodhi_already_commented() to detected changed updates. Currently it doesn't. That's the Level 1 solution.
The issue in question now is if those new builds introduce a dependency conflict? In that case, we should note the failure in the bodhi update since the results are against newer builds
Yes, we want to post new results to bodhi because the update has changed.
(and remove any previously supplied karma </future>)?
We don't supply karma now and I don't think we will in the near future. And the far future is far ahead :)
On Fri, 2011-04-15 at 10:25 -0400, Kamil Paral wrote:
My understanding to the original design goal with depcheck was that once an update was tested and accepted ... it must not be revoked. The set of accepted updates must always be dep-free. If new builds are added to an update, those builds must represent a new set of -pending updates for testing. If another update lands in -pending, and causes problems with some previously accepted update ... the problem is with the new update, not with something already accepted.
Does this design goal still hold?
Yes, it does. Unless some update in the accepted set changes.
Even though the updates should not influence each other (all the related builds should be grouped together in a single update), it's naive to suppose it's always the case. If we presume that sometimes two different updates can influence each other (which is the main presumption of our whole depcheck testing in the first case, btw), then we have to clear the accepted set and start building it again when some already accepted update has been changed. Because it could have broken not only itself, but also some other already accepted updates.
I may be misreading, but I worry about breaking the design principle that the previously 'accepted' set is no longer valid and must be retested. Perhaps this is just confusion over wording though. I'm treating new builds to an existing update as completely new builds. They will have -pending and be in the queue for acceptance like all other -pending builds. I don't understand why clearing all previously -accepted builds is required? They've already passed testing, and at any time may be moved into updates-[testing].
Once something has been accepted, it's known good and theoretically, that build will move out to updates-testing and no longer be in our queues for verification. If a new build is added to an existing update, it is tagged with -pending and treated as an entirely new set of builds, right?
That's the Level 2 (long-term) solution.
After we have the concept of ResultDB and we abolish the concept of sending emails to maintainers for every single change in depcheck results, we can do the proper solution as described by Josef. That means emptying the 'accepted set' if some update from that set has been changed.
When new builds are added to an existing update, the previous builds may have already moved on to updates-[testing] and will be automatically included in future depcheck runs. If they aren't in updates[-testing] yet, will bodhi remove the -pending tags from the previous builds, or will there be two versions of the same package still in -pending?
I suppose that if I have update "foo-1.1,bar-1.1" and then edit the update and replace "foo-1.1" for "foo-1.2", then "foo-1.1" should be untagged from updates-testing and "foo-1.2" should be tagged into updates-testing. Confirmation with lmacken needed. I believe this is exactly the kind of issues we've been hitting lately with Bodhi (untagging wasn't done right).
Agreed, I believe this is an important assertion we need.
Thanks, James
I may be misreading, but I worry about breaking the design principle that the previously 'accepted' set is no longer valid and must be retested. Perhaps this is just confusion over wording though. I'm treating new builds to an existing update as completely new builds. They will have -pending and be in the queue for acceptance like all other -pending builds. I don't understand why clearing all previously -accepted builds is required? They've already passed testing, and at any time may be moved into updates-[testing].
There is a slight difference between adding a build and replacing a build (by a newer version). I was talking about the latter one. But in the end it doesn't matter I guess.
I understand your concern and proposed workflow. But depcheck.py doesn't operate on builds, it operates on updates (as opposed to the depcheck script itself). So let's have a hypothetical update:
foo,bar (update) ======= foo (build) bar (build)
When we detect accepted builds, we detect comment for "foo,bar" and hence we proclaim foo and bar as accepted. When we mark some builds as accepted, we mark the whole update, and that makes all builds inside it accepted.
We don't operate on builds. We simply don't have any approach how to distinguish that foo is accepted, but bar is not (e.g. because it was added later). We do per-update testing. Now. We could probably do per-build testing once we have ResultsDB. Then your approach would be possible.
Still, two problems remain: 1. Your approach works when 'bar' is added to the update. But if I replace 'bar-1' by 'bar-2', I can't claim 'foo' is accepted. It can have broken dependency now. => If one update may become broken, other updates may become broken too.
2. Even if we could claim that 'foo' is accepted and 'bar' is not, how the RelEng team can push the update? Does it make even sense?
What exactly is wrong about emptying the accepted set, if we don't spam maintainers? Of course I don't want to empty the set if it is not needed, but if we change an already accepted update, I don't see another way.
Also, I think in the future we won't interact with maintainers much. I think depcheck and upgradepath will be mainly RelEng-related tests ("give the proposed updates subset I can push"). The package maintainers will just receive some notice like "Your update was not pushed because...". But those details are not much important right now.
On Fri, 2011-04-15 at 11:52 -0400, Kamil Paral wrote:
I may be misreading, but I worry about breaking the design principle that the previously 'accepted' set is no longer valid and must be retested. Perhaps this is just confusion over wording though. I'm treating new builds to an existing update as completely new builds. They will have -pending and be in the queue for acceptance like all other -pending builds. I don't understand why clearing all previously -accepted builds is required? They've already passed testing, and at any time may be moved into updates-[testing].
There is a slight difference between adding a build and replacing a build (by a newer version). I was talking about the latter one. But in the end it doesn't matter I guess.
I understand your concern and proposed workflow. But depcheck.py doesn't operate on builds, it operates on updates (as opposed to the depcheck script itself). So let's have a hypothetical update:
Ah yes, I am confused by that. Is the following accurate of the flow?
1. post-koji-build-batch provides a list of builds and schedules depcheck job 2. depcheck.py wrapper calls depcheck test against builds 3. depcheck.py wrapper determines what updates each build maps to 4. depcheck.py wrapper provides feedback for each update based on the net result of builds tested in that update
foo,bar (update)
foo (build) bar (build)
When we detect accepted builds, we detect comment for "foo,bar" and hence we proclaim foo and bar as accepted. When we mark some builds as accepted, we mark the whole update, and that makes all builds inside it accepted.
We don't operate on builds. We simply don't have any approach how to distinguish that foo is accepted, but bar is not (e.g. because it was added later). We do per-update testing. Now. We could probably do per-build testing once we have ResultsDB. Then your approach would be possible.
Still, two problems remain:
- Your approach works when 'bar' is added to the update. But if I replace 'bar-1' by 'bar-2', I can't claim 'foo' is accepted. It can have broken dependency now.
=> If one update may become broken, other updates may become broken too.
If the foo-1.1 and bar-1.1 builds pass depcheck ... we'll want those builds to move out of -pending immediately (or on next push from rel-eng).
If the update is later changed with foo-1.2 (BAD) and bar-1.1, foo-1.2 will be noted as breaking deps, and the update will be commented with a FAILED. However, we have to assume that foo-1.1 and bar-1.1 have already been moved out of -pending and could already be in 'updates-testing'. This scenario seems fine though, since it will keep foo-1.2 in -pending ... preventing the accepted+updates-testing+updates +stable set from breaking repoclosure. To resolve this, the maintainer needs to address the repoclosure problem, which may involve ...
1. Build foo-1.3 to resolve broken dep introduced 2. Build bar-1.2 to satisfy the dep 3. Contact maintainer of another package that needs to be updated 4. Contact rel-eng requesting they remove foo-1.1 and bar-1.1 from updates-testing
Open question - How does bodhi handle -pending tags for builds already in an updated when the update 1) gets a new package added, 2) gets updated packages?
- Even if we could claim that 'foo' is accepted and 'bar' is not, how the RelEng team can push the update? Does it make even sense?
What exactly is wrong about emptying the accepted set, if we don't spam maintainers? Of course I don't want to empty the set if it is not needed, but if we change an already accepted update, I don't see another way.
It's a change in the original design of the test, which may be needed, but makes me nervous as to whether we are breaking any assumptions held throughout the code. I remember having long talks w/ wwoods about never pulling previously accepted builds back into t mix. In some scenarios we discussed, it may not have been the exactly correct way to handle it, but we always maintained repoclosure across accepted + updates-testing + updates + stable.
I think I'm holding on to that original concept because rel-eng pushing is asynchronous. We have no idea when they will take the accepted set and move them to updates-testing. The idea of subtracting (not just adding) to the accepted set seems to expose this to pushing broken updates depending when rel-eng does their pushing.
Also, I think in the future we won't interact with maintainers much. I think depcheck and upgradepath will be mainly RelEng-related tests ("give the proposed updates subset I can push"). The package maintainers will just receive some notice like "Your update was not pushed because...". But those details are not much important right now.
Thanks, James
On 04/15/2011 10:29 AM, James Laska wrote:
On Fri, 2011-04-15 at 11:52 -0400, Kamil Paral wrote:
I may be misreading, but I worry about breaking the design principle that the previously 'accepted' set is no longer valid and must be retested. Perhaps this is just confusion over wording though. I'm treating new builds to an existing update as completely new builds. They will have -pending and be in the queue for acceptance like all other -pending builds. I don't understand why clearing all previously -accepted builds is required? They've already passed testing, and at any time may be moved into updates-[testing].
There is a slight difference between adding a build and replacing a build (by a newer version). I was talking about the latter one. But in the end it doesn't matter I guess.
I understand your concern and proposed workflow. But depcheck.py doesn't operate on builds, it operates on updates (as opposed to the depcheck script itself). So let's have a hypothetical update:
Ah yes, I am confused by that. Is the following accurate of the flow?
1. post-koji-build-batch provides a list of builds and schedules depcheck job 2. depcheck.py wrapper calls depcheck test against builds 3. depcheck.py wrapper determines what updates each build maps to 4. depcheck.py wrapper provides feedback for each update based on the net result of builds tested in that update
Slight modification: 1. post-koji-build-batch provides a list of builds and schedules depcheck job 2. depcheck.py downloads all builds in the *-pending tag passed in 3. depcheck.py determines which builds are already passed by querying bodhi for each update 4. depcheck.py wrapper calls depcheck test against the downloaded and classified builds 5. depcheck.py wrapper determines what updates each build maps to 6. depcheck.py wrapper provides feedback for each update based on the net result of builds tested in that update
foo,bar (update)
foo (build) bar (build)
When we detect accepted builds, we detect comment for "foo,bar" and hence we proclaim foo and bar as accepted. When we mark some builds as accepted, we mark the whole update, and that makes all builds inside it accepted.
We don't operate on builds. We simply don't have any approach how to distinguish that foo is accepted, but bar is not (e.g. because it was added later). We do per-update testing. Now. We could probably do per-build testing once we have ResultsDB. Then your approach would be possible.
Still, two problems remain:
- Your approach works when 'bar' is added to the update. But if I replace 'bar-1' by 'bar-2', I can't claim 'foo' is accepted. It can have broken dependency now.
=> If one update may become broken, other updates may become broken too.
If the foo-1.1 and bar-1.1 builds pass depcheck ... we'll want those builds to move out of -pending immediately (or on next push from rel-eng).
If the update is later changed with foo-1.2 (BAD) and bar-1.1, foo-1.2 will be noted as breaking deps, and the update will be commented with a FAILED. However, we have to assume that foo-1.1 and bar-1.1 have already been moved out of -pending and could already be in 'updates-testing'. This scenario seems fine though, since it will keep foo-1.2 in -pending ... preventing the accepted+updates-testing+updates +stable set from breaking repoclosure. To resolve this, the maintainer needs to address the repoclosure problem, which may involve ...
1. Build foo-1.3 to resolve broken dep introduced 2. Build bar-1.2 to satisfy the dep 3. Contact maintainer of another package that needs to be updated 4. Contact rel-eng requesting they remove foo-1.1 and bar-1.1 from updates-testing
Does this mean that a partial update would be pushed (ie foo-1 and bar-1 will be pushed foo-1.2 would be left behind?
Open question - How does bodhi handle -pending tags for builds already in an updated when the update 1) gets a new package added, 2) gets updated packages?
- Even if we could claim that 'foo' is accepted and 'bar' is not, how the RelEng team can push the update? Does it make even sense?
What exactly is wrong about emptying the accepted set, if we don't spam maintainers? Of course I don't want to empty the set if it is not needed, but if we change an already accepted update, I don't see another way.
It's a change in the original design of the test, which may be needed, but makes me nervous as to whether we are breaking any assumptions held throughout the code. I remember having long talks w/ wwoods about never pulling previously accepted builds back into t mix. In some scenarios we discussed, it may not have been the exactly correct way to handle it, but we always maintained repoclosure across accepted + updates-testing + updates + stable.
I think I'm holding on to that original concept because rel-eng pushing is asynchronous. We have no idea when they will take the accepted set and move them to updates-testing. The idea of subtracting (not just adding) to the accepted set seems to expose this to pushing broken updates depending when rel-eng does their pushing.
I'm not disagreeing with you here, but I thought that rel-eng just pushed updates, not builds.
If a package in an update breaks, that update shouldn't be pushed and could affect other updates that are pending.
If we did revoke 'accepted-ness' from an update's build, the burden should still be on the maintainer of that update to fix it but I think that a more interesting question is: "What happens when an accepted update depends on non-broken builds in a failed update?". Can that happen? Wouldn't that still break repoclosure if the passed update was pushed without the non-broken builds it depends on from the failed update?
I'm still wondering if dependencies should be checked at push time (with different comment structure, if any comments are left) instead of just at various times before push.
Also, I think in the future we won't interact with maintainers much. I think depcheck and upgradepath will be mainly RelEng-related tests ("give the proposed updates subset I can push"). The package maintainers will just receive some notice like "Your update was not pushed because...". But those details are not much important right now.
Thanks, James
Tim
On Sun, 2011-04-17 at 22:12 -0600, Tim Flink wrote:
On 04/15/2011 10:29 AM, James Laska wrote:
On Fri, 2011-04-15 at 11:52 -0400, Kamil Paral wrote:
<snip>
I understand your concern and proposed workflow. But depcheck.py doesn't operate on builds, it operates on updates (as opposed to the depcheck script itself). So let's have a hypothetical update:
Ah yes, I am confused by that. Is the following accurate of the flow?
1. post-koji-build-batch provides a list of builds and schedules depcheck job 2. depcheck.py wrapper calls depcheck test against builds 3. depcheck.py wrapper determines what updates each build maps to 4. depcheck.py wrapper provides feedback for each update based on the net result of builds tested in that update
Slight modification: 1. post-koji-build-batch provides a list of builds and schedules depcheck job 2. depcheck.py downloads all builds in the *-pending tag passed in 3. depcheck.py determines which builds are already passed by querying bodhi for each update 4. depcheck.py wrapper calls depcheck test against the downloaded and classified builds 5. depcheck.py wrapper determines what updates each build maps to 6. depcheck.py wrapper provides feedback for each update based on the net result of builds tested in that update
Quite a wrapper!
foo,bar (update)
foo (build) bar (build)
When we detect accepted builds, we detect comment for "foo,bar" and hence we proclaim foo and bar as accepted. When we mark some builds as accepted, we mark the whole update, and that makes all builds inside it accepted.
We don't operate on builds. We simply don't have any approach how to distinguish that foo is accepted, but bar is not (e.g. because it was added later). We do per-update testing. Now. We could probably do per-build testing once we have ResultsDB. Then your approach would be possible.
Still, two problems remain:
- Your approach works when 'bar' is added to the update. But if I replace 'bar-1' by 'bar-2', I can't claim 'foo' is accepted. It can have broken dependency now.
=> If one update may become broken, other updates may become broken too.
If the foo-1.1 and bar-1.1 builds pass depcheck ... we'll want those builds to move out of -pending immediately (or on next push from rel-eng).
If the update is later changed with foo-1.2 (BAD) and bar-1.1, foo-1.2 will be noted as breaking deps, and the update will be commented with a FAILED. However, we have to assume that foo-1.1 and bar-1.1 have already been moved out of -pending and could already be in 'updates-testing'. This scenario seems fine though, since it will keep foo-1.2 in -pending ... preventing the accepted+updates-testing+updates +stable set from breaking repoclosure. To resolve this, the maintainer needs to address the repoclosure problem, which may involve ...
1. Build foo-1.3 to resolve broken dep introduced 2. Build bar-1.2 to satisfy the dep 3. Contact maintainer of another package that needs to be updated 4. Contact rel-eng requesting they remove foo-1.1 and bar-1.1 from updates-testing
Does this mean that a partial update would be pushed (ie foo-1 and bar-1 will be pushed foo-1.2 would be left behind?
In the hypothetical example I posted, yeah, foo-1 and bar-1 would have been accepted and moved out of our hands. foo-1.2, which introduces the breakage, would be rejected until something comes along to resolve it.
Open question - How does bodhi handle -pending tags for builds already in an updated when the update 1) gets a new package added, 2) gets updated packages?
- Even if we could claim that 'foo' is accepted and 'bar' is not, how the RelEng team can push the update? Does it make even sense?
What exactly is wrong about emptying the accepted set, if we don't spam maintainers? Of course I don't want to empty the set if it is not needed, but if we change an already accepted update, I don't see another way.
It's a change in the original design of the test, which may be needed, but makes me nervous as to whether we are breaking any assumptions held throughout the code. I remember having long talks w/ wwoods about never pulling previously accepted builds back into t mix. In some scenarios we discussed, it may not have been the exactly correct way to handle it, but we always maintained repoclosure across accepted + updates-testing + updates + stable.
I think I'm holding on to that original concept because rel-eng pushing is asynchronous. We have no idea when they will take the accepted set and move them to updates-testing. The idea of subtracting (not just adding) to the accepted set seems to expose this to pushing broken updates depending when rel-eng does their pushing.
I'm not disagreeing with you here, but I thought that rel-eng just pushed updates, not builds.
Ah, you may be correct. We'd have to confirm with rel-eng. According to https://fedoraproject.org/wiki/Pushing_fedora_updates#Get_a_list_of_packages..., they run bodhi with the -P option (-P, --push Display and push any pending updates (releng only)). Hmm, so what does "pending updates" mean?
If a package in an update breaks, that update shouldn't be pushed and could affect other updates that are pending.
I see how pushing builds vs updates could be confusing (and possibly messy).
If we did revoke 'accepted-ness' from an update's build, the burden should still be on the maintainer of that update to fix it
definitely
but I think that a more interesting question is: "What happens when an accepted update depends on non-broken builds in a failed update?". Can that happen? Wouldn't that still break repoclosure if the passed update was pushed without the non-broken builds it depends on from the failed update?
I'm still wondering if dependencies should be checked at push time (with different comment structure, if any comments are left) instead of just at various times before push.
Yeah maybe, when the time comes to move depcheck into enforcing mode, we'll likely need to coordinate with rel-eng on the procedure they use to push updates.
Thanks, James
On 04/15/2011 03:40 AM, Kamil Paral wrote:
I wonder how hard it would be to change the way depcheck and bodhi work a little bit.
Instead of just scraping comments, another option would be to have a text field in bodhi that held the list of passing tests. When a test passes, it adds its name to the field. When an update is changed, that field is reset and the tests are re-run.
That would be a nice solution if we didn't have ResultsDB in our plan.
Do we have any documentation on what ResultsDB is supposed to do other than store and display test results?
I've found several wiki pages about ResultsDB ([1], [2], [3]) and some old emails ([4], [5]) but I have yet to find any references to ResultsDB being used inside AutoQA tests for anything other than reporting.
Granted, it isn't a stretch to see ResultsDB used that way but I dislike making assumptions about what a given feature is going to do - it leads to "feature XXX is a magical cure-all that will solve all of our problems if we could only get it implemented" types of thinking.
I'm not saying that ResultsDB has become one of those mystical, cure-all features. I'm just noting that _I_ know very little about it or what it supposed to do other than function as a front-end for test results. I'll spend some more quality time with google and the code that I found [6] to see if I can understand a bit more.
[1] http://fedoraproject.org/wiki/AutoQA_resultsdb_use_cases [2] http://fedoraproject.org/wiki/AutoQA_resultsdb_API [3] https://fedoraproject.org/wiki/AutoQA_resultsdb_approaches [4] https://fedorahosted.org/pipermail/autoqa-devel/2010-February/000201.html [5] http://web.archiveorange.com/archive/v/BwoyykzB13b8eDTlV1TC [6] http://www.assembla.com/code/resultdb/git/nodes
It might make more sense to wait for resultsdb on this one, I'm not sure. Either one but might be more accurate and faster than trying to interpret comments.
Sending bodhi comments was just a quick solution for letting maintainers know. Instead of spending time on further temporary hacks (like hacking into Bodhi something that won't be needed soon) I see as a better solution to spend the time on the proper solution - ResultDB. It's not hard, it just needs focus.
I agree that ResultsDB isn't an incredibly difficult concept and could be finished if we focused on it. If these things are already planned for ResultsDB, then requesting extra bodhi/koji modifications doesn't make sense.
Do we know how often this is happening?
Sometimes. Maintainers change their updates every now and then. Level 1 solution (re-test just the changed update) is extremely easy to implement and eliminates a lot of problems. So it's reasonable to spend a few hours and have it done.
Fair enough. If it doesn't happen all that often, we probably don't need to spend all that much time on it.
=== RelEng workflow ===
At the moment, rel-eng is pushing updates manualy few times a week [citation needed, this is informal information which has not been audited].
The thing is, that we can provide the _at the moment 'pushable'_ subset of packages. When rel-eng will want to push from -pending to {stable, updates, ...}, they'll just 'list' the pushable subset (yes, resultsdb will help ;-)).
"The tool" will detect if there were any changes in the pushable subset, and "the tool" will inform rel-eng that they either need to wait for the results of the next depcheck run, or (if we're able to do it) request new depcheck run.
Why not just make a tool that re-reruns all "*-pending" updates through a battery of tests (depcheck and upgradepath ATM), ignoring any "PASSED" results and not posting any new comments to bodhi unless there was a change for an update? That way we don't have to worry about detecting if there were changes or not and we can be more confidant of the to-be-pushed set as a whole (which is what our end goal is, IIRC)
That's exactly what upgradepath does now. Depcheck is little different, it uses the concept of 'accepted updates'.
We can't remove the 'accepted updates' concept from depcheck, because that could cause a flood of emails going to maintainers' mailboxes through bodhi comments. If we don't use the concept of ever-growing accepted set of updates, it might happen that your update will be accepted at 1 PM, rejected at 2 PM (someone pushed conflicting update), accepted at 3 PM (someone removed the conflicting update), etc. Hence the Level 1 solution, which is not perfect, but doesn't spam maintainers.
Hence the part about not changing bodhi comments when re-running the tests in this pre-push scenario.
After we have the concept of ResultDB and we abolish the concept of sending emails to maintainers for every single change in depcheck results, we can do the proper solution as described by Josef. That means emptying the 'accepted set' if some update from that set has been changed. It also means providing the latest never-outdated results directly to RelEng (or anyone else interested) on request, not continuously to package maintainers. If _in that moment of RelEng push_ some update is excluded because of failed dependencies, only after that we can send email to the maintainer. (Or some similar approach).
I don't really understand why using bodhi comments and re-running depcheck are mutually exclusive. I don't think that it would be that hard to add a parameter to depcheck that would test everything in a tag and create a report without updating bodhi.
Then again, if this isn't a big problem then it might not be worth a whole lot of effort right now. Is this something requested by rel-eng? Is it something that they would want or use?
To sum it up, the really proper solution is not possible as long as we rely on sending emails to maintainers for every change. That's not viable, just temporary, because we could easily turn into spammers.
Tim
Hi,
----- Original Message -----
From: "Tim Flink" tflink@redhat.com
<snipped> Do we have any documentation on what ResultsDB is supposed to do other than store and display test results?
I've found several wiki pages about ResultsDB ([1], [2], [3]) and some old emails ([4], [5]) but I have yet to find any references to ResultsDB being used inside AutoQA tests for anything other than reporting.
Granted, it isn't a stretch to see ResultsDB used that way but I dislike making assumptions about what a given feature is going to do - it leads to "feature XXX is a magical cure-all that will solve all of our problems if we could only get it implemented" types of thinking.
<snipped>
I'm currently working on a ResultsDB resumé https://fedoraproject.org/wiki/User:Jskladan/Sandbox:ResultsDB_Writeup (work in progress though). Storing results is probably the least important role for ResultsDB. The main purpose of resultsdb is to provide data source for various 'frontends' via easy-to-use querying API. So this is the reason why we want to wait for ResultsDB. Once we have the data in, we can wery easily make a frontend, which will present 'Subset of updates which can be pushed now' to Rel-Eng. We'll take the latests Depcheck result, check whether some of the updates changed (which would discard either the respective update or the whole result as such).
On 04/18/2011 08:10 AM, Josef Skladanka wrote:
Hi,
----- Original Message -----
From: "Tim Flink" tflink@redhat.com <snipped> Do we have any documentation on what ResultsDB is supposed to do other than store and display test results?
I've found several wiki pages about ResultsDB ([1], [2], [3]) and some old emails ([4], [5]) but I have yet to find any references to ResultsDB being used inside AutoQA tests for anything other than reporting.
Granted, it isn't a stretch to see ResultsDB used that way but I dislike making assumptions about what a given feature is going to do - it leads to "feature XXX is a magical cure-all that will solve all of our problems if we could only get it implemented" types of thinking.
<snipped>
I'm currently working on a ResultsDB resumé https://fedoraproject.org/wiki/User:Jskladan/Sandbox:ResultsDB_Writeup (work in progress though). Storing results is probably the least important role for ResultsDB. The main purpose of resultsdb is to provide data source for various 'frontends' via easy-to-use querying API. So this is the reason why we want to wait for ResultsDB. Once we have the data in, we can wery easily make a frontend, which will present 'Subset of updates which can be pushed now' to Rel-Eng. We'll take the latests Depcheck result, check whether some of the updates changed (which would discard either the respective update or the whole result as such).
Thanks for the link, I appreciate it. After I sent my previous email, I noticed that the last commit in the repo I found was in April 2010, not 2011 so the updated code is appreciated.
Tim
autoqa-devel@lists.fedorahosted.org