#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 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 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 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 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 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):

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 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: 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