Let's resolve #88 first so we can rebase this to get the state_reason stuff right.

EDIT: I take it back. We could merge this one first, but see the comment below about error handling which probably needs to be resolved first.

What if bool(m) == False? Should this raise an error?

If not, it is probably worth explaining the logic in a comment for future readers.

Set the expiration time explicitly to something here. It can be longer than the other cache in #99?

Wait - this gets me thinking. I'll add a longer comment on the PR itself.

The error handling approach here may need to be changed.

It occurred to me when I looked at the configuration of region. I thought: well, we're caching info from koji associated with an nvr. That will never change. Those builds are immutable. We can cache forever!

And then I looked at the error handling. The function looks to see first if the build exists at all in koji, and if it does not, then it returns a dict explaining the error. This will be cached. Then, if the build finally does show up in koji, we will keep returning this incorrect cached error result.

The better way to do it (imho) is to replace all the error handling here with raising exceptions.

This way, anytime there's an error, nothing will be cached.

Multiple calls to this function will raise an exception every time until the nvr finally shows up in koji, at which point it will cache the real and good value. This can be cached for a long time.

Am I on the right track?

@ralph: _get_additional_data_from_koji(...) is now raising an exception.

I'm an idiot today. I merged #88 with the .transition stuff before this.

Now, this (I guess) needs to be rebased on that and have this section of code changed to use .transition calls.

Looks good now.

• Sorry about merging #88 first. That was a mistake. I guess this PR still needs to have the state and state_reason statements modified to use .transition(..) now?

After that, :+1:.