Take everything in this except block and move it to its own function called report_failure or something so that this main function can remain high level with no low-level details.

Also, at the end of the except block, include a bare raise statement which will re-raise the exception so it gets printed in the logs and kills the process with a non-zero exit code.

Could this be:

message = '  No owner could be found for username %s' % issue.downstream.get('owner')
log.warning(message.strip())
raise ValueError(message)

... so that the reason propagates to the error email?

I don't see where issue gets assigned here. Some error in refactoring?

I also don't see where admin_template gets used below here. Perhaps it isn't needed?

If it isn't needed, perhaps you can dispense with the whole setup here to query Jira to find more info about the admins in the first place. You could just keep the list of admins in the config as a list of emails, and email them directly. It would be much more simple code in this error case.

What would happen if sync2jira failed in the inner loop because there was suddenly some problem trying to access jira itself. The email code here would fail as well, and we wouldn't be notified.

Better to streamline this function. WDYT?

Could this be:
message = ' No owner could be found for username %s' % issue.downstream.get('owner')
log.warning(message.strip())
raise ValueError(message)

... so that the reason propagates to the error email?

Yes, I like this better.

I don't see where issue gets assigned here. Some error in refactoring?

Yeah needs to be removed.

I also don't see where admin_template gets used below here. Perhaps it isn't needed?
If it isn't needed, perhaps you can dispense with the whole setup here to query Jira to find more info about the admins in the first place. You could just keep the list of admins in the config as a list of emails, and email them directly. It would be much more simple code in this error case.
What would happen if sync2jira failed in the inner loop because there was suddenly some problem trying to access jira itself. The email code here would fail as well, and we wouldn't be notified.
Better to streamline this function. WDYT?

I agree its easier. But I wanted to keep it so that when we create the duplicate email we can have the admins names so they're easier to get in touch with. Maybe have both (username + email), that way we can just use email for when we are sending the failure notice? Take a look at the most recent PR and let me know what you think.

Looks better, but this whole section with jira_instance and client can probably be removed too?

Looks better, but this whole section with jira_instance and client can probably be removed too?

Yup my mistake. @ralph final check?

Thanks! :+1: