#303: depcheck: error: Incorrect number of arguments ----------------------+----------------------------------------------------- Reporter: kparal | Owner: Type: defect | Status: new Priority: critical | Milestone: 0.4.7 Component: tests | Keywords: ----------------------+----------------------------------------------------- Our old friend CmdError, but this time seems a different one:
https://fedorahosted.org/pipermail/autoqa-results/2011-April/103714.html
#303: depcheck: error: Incorrect number of arguments ----------------------+----------------------------------------------------- Reporter: kparal | Owner: tflink Type: defect | Status: assigned Priority: critical | Milestone: 0.4.7 Component: tests | Resolution: Keywords: | ----------------------+----------------------------------------------------- Changes (by tflink):
* owner: => tflink * status: new => assigned
#303: depcheck: error: Incorrect number of arguments ----------------------+----------------------------------------------------- Reporter: kparal | Owner: tflink Type: defect | Status: assigned Priority: critical | Milestone: 0.4.7 Component: tests | Resolution: Keywords: | ----------------------+----------------------------------------------------- Comment (by tflink):
This looks to happen whenever we get a job that is completely accepted builds. ATM, depcheck needs to have at least one build to test or it will crash.
In my mind, the q-n-d fix for this would be to have depcheck run without a non-zero exit code when it doesn't get any builds to test.
The better fix would probably be to not schedule depcheck runs when there are no builds to test (i.e. when all the builds are accepted).
#303: depcheck: error: Incorrect number of arguments ----------------------+----------------------------------------------------- Reporter: kparal | Owner: tflink Type: defect | Status: assigned Priority: critical | Milestone: 0.4.7 Component: tests | Resolution: Keywords: | ----------------------+----------------------------------------------------- Comment (by kparal):
My question is: If all builds are accepted, then why the depcheck test was even triggered?
Possible solution: If someone edits an update, it re-applies the -updates- pending tag, but we still detect it as accepted, if it passed previously? This way it can happen that everything "is accepted".
Replying to [comment:2 tflink]:
In my mind, the q-n-d fix for this would be to have depcheck run without
a non-zero exit code when it doesn't get any builds to test.
Easier solution would be probably to detect this in depcheck.py and not run the command at all, just end the test with NEEDS_INSPECTION.
#303: depcheck: error: Incorrect number of arguments ----------------------+----------------------------------------------------- Reporter: kparal | Owner: tflink Type: defect | Status: assigned Priority: critical | Milestone: 0.4.7 Component: tests | Resolution: Keywords: | ----------------------+----------------------------------------------------- Comment (by tflink):
Replying to [comment:3 kparal]:
My question is: If all builds are accepted, then why the depcheck test
was even triggered?
The test is scheduled because we don't determine the package's status until after the test is scheduled.
Possible solution: If someone edits an update, it re-applies the
-updates-pending tag, but we still detect it as accepted, if it passed previously? This way it can happen that everything "is accepted".
Hmm, that might be a different issue that we need to look into; I don't think that we handle that case right now. Currently, we get all accepted builds if our timing is right - ie, no new builds since last run, everything has passed so far and updates haven't been pushed to stable yet.
Replying to [comment:2 tflink]:
In my mind, the q-n-d fix for this would be to have depcheck run
without a non-zero exit code when it doesn't get any builds to test.
Easier solution would be probably to detect this in depcheck.py and not
run the command at all, just end the test with NEEDS_INSPECTION.
I don't think that either solution would be difficult to implement. I already have a 2 line patch up in review board to keep depcheck from exiting with non-zero status when all the builds are accepted - https://fedorahosted.org/reviewboard/r/135/
I didn't think about the update editing case, though so I'm not as sure that this alone is the best solution as I was 20 minutes ago.
I propose that we change depcheck for now and create a new ticket to handle the edited update use case.
#303: depcheck: error: Incorrect number of arguments ----------------------+----------------------------------------------------- Reporter: kparal | Owner: tflink Type: defect | Status: assigned Priority: critical | Milestone: 0.4.7 Component: tests | Resolution: Keywords: | ----------------------+----------------------------------------------------- Comment (by kparal):
Replying to [comment:4 tflink]:
Currently, we get all accepted builds if our timing is right - ie, no
new builds since last run, everything has passed so far and updates haven't been pushed to stable yet.
no new updates since last run => koji watcher should not trigger any post- bodhi-update or post-bodhi-update-batch events => no depcheck should be scheduled. Either I'm still missing something or something is wrong here.
#303: depcheck: error: Incorrect number of arguments ----------------------+----------------------------------------------------- Reporter: kparal | Owner: tflink Type: defect | Status: assigned Priority: critical | Milestone: 0.4.7 Component: tests | Resolution: Keywords: | ----------------------+----------------------------------------------------- Comment (by tflink):
Replying to [comment:5 kparal]:
no new updates since last run => koji watcher should not trigger any
post-bodhi-update or post-bodhi-update-batch events => no depcheck should be scheduled. Either I'm still missing something or something is wrong here.
Good point on the scheduling.
I dug into this farther and the issue is a timing/coordination issue due to how depcheck pulls in packages. Basically, depcheck doesn't limit itself to the nvrs passed in to the test - it pulls in everything from *-pending and tests what isn't already accepted.
The timeline (in UTC, all on 2011-04-11) is:
* 13:10:53 - depcheck-x86_64 (job 80736) scheduled for dist-f14-updates- testing-pending * Triggered by new builds for fuse-encfs-1.7.4-1.fc14 and 1:libguestfs-1.8.5-1.fc14 * 13:32:25 - sugar-paint-32-1.fc14 tagged dist-f14-updates-testing- pending by bodhi * 13:35:22 - job 80736 queued, depcheck pulls in all pending builds (including sugar-paint) * 13:37:15 - sugar-paint-32-1.fc14 passes depcheck, bodhi comment created * 13:40:50 - depcheck-x86_64 (job 80752) scheduled for dist-f14-updates- testing-pending * Triggered by build for sugar-paint-32-1.fc14 * 13:45:45 - job 80752 queued * 13:46:19 - job 80742 fails because all builds for dist-f14-updates- testing-pending are accepted
Given this new information, the possible fixes I see are: * Change depcheck.py so that it doesn't pull in any pending packages that weren't specified * Change depcheck so that it doesn't fail when no pending packages are specified
#303: depcheck: error: Incorrect number of arguments ----------------------+----------------------------------------------------- Reporter: kparal | Owner: tflink Type: defect | Status: assigned Priority: critical | Milestone: 0.4.7 Component: tests | Resolution: Keywords: | ----------------------+----------------------------------------------------- Comment (by jskladan):
Replying to [comment:6 tflink]:
I dug into this farther and the issue is a timing/coordination issue due
to how depcheck pulls in packages. Basically, depcheck doesn't limit itself to the nvrs passed in to the test - it pulls in everything from *-pending and tests what isn't already accepted.
The timeline (in UTC, all on 2011-04-11) is:
- 13:10:53 - depcheck-x86_64 (job 80736) scheduled for dist-f14
-updates-testing-pending
- Triggered by new builds for fuse-encfs-1.7.4-1.fc14 and
1:libguestfs-1.8.5-1.fc14
- 13:32:25 - sugar-paint-32-1.fc14 tagged dist-f14-updates-testing-
pending by bodhi
- 13:35:22 - job 80736 queued, depcheck pulls in all pending builds
(including sugar-paint)
- 13:37:15 - sugar-paint-32-1.fc14 passes depcheck, bodhi comment
created
- 13:40:50 - depcheck-x86_64 (job 80752) scheduled for dist-f14
-updates-testing-pending
- Triggered by build for sugar-paint-32-1.fc14
- 13:45:45 - job 80752 queued
- 13:46:19 - job 80742 fails because all builds for dist-f14-updates-
testing-pending are accepted
Great research! This was what I thought was happening (on the scheduling front), great to know that for sure.
Given this new information, the possible fixes I see are:
- Change depcheck.py so that it doesn't pull in any pending packages
that weren't specified
- Change depcheck so that it doesn't fail when no pending packages are
specified
I'd prefer the second possibility. If nothing else, it seems better to run depcheck just once, than to download the whole -pending tag just to test one package, which could have been already tested in the previous run.
#303: depcheck: error: Incorrect number of arguments ----------------------+----------------------------------------------------- Reporter: kparal | Owner: tflink Type: defect | Status: assigned Priority: critical | Milestone: 0.4.7 Component: tests | Resolution: Keywords: | ----------------------+----------------------------------------------------- Comment (by tflink):
Replying to [comment:7 jskladan]:
I'd prefer the second possibility. If nothing else, it seems better to
run depcheck just once, than to download the whole -pending tag just to test one package, which could have been already tested in the previous run.
After thinking about it a bit more, I'd like to propose a slightly different fix for the sort term: 1. Patch depcheck to not return a non-zero exit code if there are no pending builds * Submitted to reviewboard: https://fedorahosted.org/reviewboard/r/135/ 1. Patch depcheck.py so that it doesn't run ./depcheck if there are no pending builds * Submitted to reviewboard: https://fedorahosted.org/reviewboard/r/135/ * '''NOTE''': This patch isn't done. It does work in the strictest sense but messes with the reporting in a bad way. If we want to go this route, I'll fix it up and re-submit a proper fix tomorrow.
I figure that it would be good to have both as a short term fix. That way, both the command ./depcheck and the code that coordinates depcheck runs aren't behaving badly with empty inputs.
In the long term, I think that a fix for #248 could also fix this such that we don't have to worry about scheduling overlaps.
#303: depcheck: error: Incorrect number of arguments ----------------------+----------------------------------------------------- Reporter: kparal | Owner: tflink Type: defect | Status: assigned Priority: critical | Milestone: 0.4.7 Component: tests | Resolution: Keywords: | ----------------------+----------------------------------------------------- Comment (by kparal):
Patch 135 looks good. For the other fix you provided the same link to patch 135. I found patch 136, but you probably mistakenly provided the same fix also in this patch: https://fedorahosted.org/reviewboard/r/136/
Please update the patch. According to your description I agree with going that route.
#303: depcheck: error: Incorrect number of arguments ----------------------+----------------------------------------------------- Reporter: kparal | Owner: tflink Type: defect | Status: assigned Priority: critical | Milestone: 0.4.7 Component: tests | Resolution: Keywords: | ----------------------+----------------------------------------------------- Comment (by tflink):
Replying to [comment:9 kparal]:
Patch 135 looks good. For the other fix you provided the same link to
patch 135. I found patch 136, but you probably mistakenly provided the same fix also in this patch:
https://fedorahosted.org/reviewboard/r/136/
Please update the patch. According to your description I agree with
going that route.
Bah, I uploaded the wrong file and didn't check the diff before submitting. The "correct" diff is up at https://fedorahosted.org/reviewboard/r/136/ but the same issue with reporting is still there.
#303: depcheck: error: Incorrect number of arguments ----------------------+----------------------------------------------------- Reporter: kparal | Owner: tflink Type: defect | Status: assigned Priority: critical | Milestone: 0.4.7 Component: tests | Resolution: Keywords: | ----------------------+----------------------------------------------------- Comment (by tflink):
Updated the patch again so that it works well with the summary and doesn't just kill the process.
https://fedorahosted.org/reviewboard/r/136/
#303: depcheck: error: Incorrect number of arguments ----------------------+----------------------------------------------------- Reporter: kparal | Owner: tflink Type: defect | Status: assigned Priority: critical | Milestone: 0.4.7 Component: tests | Resolution: Keywords: | ----------------------+----------------------------------------------------- Comment (by kparal):
Thanks, reviewed.
#303: depcheck: error: Incorrect number of arguments ----------------------+----------------------------------------------------- Reporter: kparal | Owner: tflink Type: defect | Status: assigned Priority: critical | Milestone: 0.4.7 Component: tests | Resolution: Keywords: | ----------------------+----------------------------------------------------- Comment (by tflink):
Replying to [comment:12 kparal]:
Thanks, reviewed.
Responded to comments in review. Will add code for problematic builds and re-submit to rb
#303: depcheck: error: Incorrect number of arguments ----------------------+----------------------------------------------------- Reporter: kparal | Owner: tflink Type: defect | Status: assigned Priority: critical | Milestone: 0.4.7 Component: tests | Resolution: Keywords: | ----------------------+----------------------------------------------------- Comment (by tflink):
Added code to handle problematic builds and posted changes in reviewboard.
https://fedorahosted.org/reviewboard/r/136/
#303: depcheck: error: Incorrect number of arguments ----------------------+----------------------------------------------------- Reporter: kparal | Owner: tflink Type: defect | Status: assigned Priority: critical | Milestone: 0.4.7 Component: tests | Resolution: Keywords: | ----------------------+----------------------------------------------------- Comment (by kparal):
Reviewed.
#303: depcheck: error: Incorrect number of arguments ----------------------+----------------------------------------------------- Reporter: kparal | Owner: tflink Type: defect | Status: assigned Priority: critical | Milestone: 0.4.7 Component: tests | Resolution: Keywords: | ----------------------+----------------------------------------------------- Comment (by tflink):
Removed excess output to the logs since this situation won't make bodhi comments and thus, people other than us are unlikely to even look at them.
Pushed to master: 706b36f5a800c8e51afc74b3fb874b48b2aea1ba
#303: depcheck: error: Incorrect number of arguments ----------------------+----------------------------------------------------- Reporter: kparal | Owner: tflink Type: defect | Status: assigned Priority: critical | Milestone: 0.4.7 Component: tests | Resolution: Keywords: | ----------------------+----------------------------------------------------- Comment (by kparal):
Thanks! May I cherry-pick to stable, or is there some reason to wait?
#303: depcheck: error: Incorrect number of arguments ----------------------+----------------------------------------------------- Reporter: kparal | Owner: tflink Type: defect | Status: closed Priority: critical | Milestone: 0.4.7 Component: tests | Resolution: fixed Keywords: | ----------------------+----------------------------------------------------- Changes (by tflink):
* status: assigned => closed * resolution: => fixed
Comment:
Replying to [comment:17 kparal]:
Thanks! May I cherry-pick to stable, or is there some reason to wait?
A reason other than a brain fart on my part, no. Pushed to stable: b79fd06d86d90aa5c4f45f307db2e5e76c6dd33e
autoqa-devel@lists.fedorahosted.org
-
fedora-badges