| Title | If the replicator detects a conflict and there's no job then it fails |
| Status | closed |
| Priority | optional |
| Assigned user | Gareth Rees |
| Organization | Ravenbrook |
| Description | If the replicator detects a conflict between a DT issue and a job, and the job doesn't exist, it fails mysteriously. This can happen if there's a bug in the replicator or the Perforce jobspec that means that the initial replication from the DT fails to make the job. It's hard for a program to tell if a job exists in Perforce, so this is a problem of robustness. Most likely to result from a problem with the configuration, or perhaps if the Perforce server is down. |
| Analysis | If the replicator detects a conflict when there's no job it tries to set P4DTI-Action to wait, but it can't because there's no job and Perforce rejects the input to p4 job -i because there's no description field. There are a number of things going wrong here. First: you have to write "raise a,b" in Python, not "raise(a,b)" -- the latter case means "raise (a,b), None". This is a major bug and needs fixing ASAP. Second, the sample database distribued with TeamTrack build 4402 had spaces at the start of state names in the STATES table. These states weren't being recognized by the TeamTrack to Perforce state map, and so a conflict was being reported. Third, if the replicator detects a conflict it tries to update the job action to "wait". But the first time it replicates, there's no job to update. So the attempted update is really a submission of a new job, and this fails because of Perforce's job spec checking. I recommend: 1. Use "raise a,b" throughout (permanent fix). 2. Report the conflict even if updating the action fields fails. So if the update fails, at least we have a record of what the conflict was. (permanent fix) 3. Don't update the action field if the job isn't being replicated -- that is, if the P4DTI-rid field of the job doesn't match the replicator id. This means that non-existent jobs (which have P4DTI-rid equal to "None") won't get updated when there's a conflict. This is a workaround that might become a permanent fix, but it needs some thought. 4. Strip initial and final spaces from state names when converting. (temporary fix). |
| How found | manual_test |
| Evidence | Traceback (innermost last): File "run_teamtrack.py", line 8, in ? r.run() File "replicator.py", line 945, in run self.poll() File "replicator.py", line 641, in poll self.replicate_many(changed_issues, changed_jobs) File "replicator.py", line 711, in replicate_many self.conflict(issue, job, message) File "replicator.py", line 603, in conflict self.update_job_action(job, 'wait') File "replicator.py", line 953, in update_job_action self.p4_run('job -i', [job]) File "replicator.py", line 594, in p4_run client = self.config['p4_client']) File "p4.py", line 63, in run raise error, results[0]['data'] Perforce error: 'Description' field blank. You must provide it. |
| Observed in | 0.3.0 |
| Created by | Gareth Rees |
| Created on | 2000-10-16 19:09:45 |
| Last modified by | Gareth Rees |
| Last modified on | 2001-12-10 18:56:32 |
| History | 2000-10-16 RB Created while testing release 0.3.0 with GDR. 2000-11-21 RB Added description and set priority to essential. 2000-12-04 RB Improved description and downgraded to "optional" because of lack of user impact. |
| Change | Effect | Date | User | Description |
|---|---|---|---|---|
| 5388 | closed | 2000-12-04 18:41:38 | Gareth Rees | Removing resolver role from the integration: Tidied up the replication and mailing methods to make this part of the replicator clearer. Changed replicate_issue_to_job() to replicate_issue_dt_to_p4() and replicate_job_to_issue() to replicate_issue_p4_to_dt() for consistency with other method names. Added new methods overwrite_issue_dt_to_p4() and overwrite_issue_p4_to_dt(), which are wrappers around the above methods that also e-mail the affected parties. In order to be able to e-mail the owner of a job, added the replicator method user_email_address for getting a Perforce user's e-mail address and the replicator configuration parameter job-owner-field that names the owner field in a job (the automatic configuration generator specifies 'Owner' for this). Removed the mail_administrator method() (use mail instead), and improved the mail() method. Improved many log entries by using issue.readable_name instead of issue.id. Re-organized and documented the replicate() function. It's no less complicated than it was before, but I think it's easier to understand. Functions like replicate() and replicate_issue_p4_to_dt() no longer return a meaningful error code. They either return or throw an exception. Changed conflict_policy so that it always returns 'dt'. This is the key change that removes the resolver's role. Changed the handling of errors to support the change in the conflict policy. There's now no need for conflict_error, so this is gone. The reverting of jobs to the corresponding issue is handled by the revert_issue_dt_to_p4() method: this ensures that it gets called in the correct place only (that is, normal replication from Perforce to the defect tracker has failed). The replicate_many() method no longer does any handling of errors. All other errors are caught by the run() method so that the replicator can keep on going, except AssertionError and KeyboardInterrupt, both of which stop the replicator. |
| 3356 | open | 2000-10-17 15:08:17 | Gareth Rees | Remove leading and trailing whitespace from state names when building tables in init_workflows method. This is a (probably temporary) workaround for job000020 and solves a problem in the supplied sample database whereby some of the state names have initial spaces. |
| 3341 | open | 2000-10-17 12:10:11 | Gareth Rees | In replicator.update_job_action() method, only update the job action if the job is being replicated by this replicator. The idea is to ensur we don't try to update non-existent jobs if a conflict is discovered before an issue has been replicated to Perforce. This is a partial and possibly permanent fix for job000020. |
| 3339 | open | 2000-10-17 12:03:14 | Gareth Rees | In the replicator.conflict() method, report the conflict even if updating the action fields fails. So if the update fails, we have a record of what the conflict was. This is a partial, permanent fix for job000020. |
| 3338 | open | 2000-10-17 11:51:06 | Gareth Rees | Use "raise a,b" rather than "raise(a,b)" -- the latter would mean "raise (a,b), None". Partial but permanent fix for job000020. |