#114 Backwards compatibility in the server for result_id argument.
Closed 2 years ago by ralph. Opened 2 years ago by ralph.

No commits found

This is tricky, because the server has to accept either a result_id or a
subject/testcase pair. We tried to decorate some blocks with comments
indicating that they can be removed in a future release.

rebased onto b6c8a45336b57a45633d4e422da05db470e5c1dd

2 years ago

Does a requests.Response have a bool coercion defined? If so, what does it do?

I would expect this to catch an exception type and raise BadRequest from that (ideally passing down some info from the exception).

Do we need another branch here for composes?

Should we just drop this code from the CLI, and pass it up to the server and let it deal with it?

By considering the performance, it would be good to use requests.Session().

If the resultsdb can't be connected or whatever any other error, we should raise it with:

response.raise_for_status()

So if we raise the error like I suggest above, we wouldn't need to check the result any more.

Would be clearer to have two versions of API for this?

  • POST /v1.0/waivers

Only accept result_id.

  • POST /v2.0/waivers

Only accept subject and testcase.

And the one day we could just deprecate the first one without touching the second one.

@gnaponie, since I'm travelling tomorrow can you take over this one? I won't make any progress.

You could pull from the git branch, push to your own fork, open a new pull request, and link to it from a comment in this.

@gnaponie, since I'm travelling tomorrow can you take over this one? I won't make any progress.
You could pull from the git branch, push to your own fork, open a new pull request, and link to it from a comment in this.

@ralph yes, sure

Metadata Update from @gnaponie:
- Request assigned

2 years ago

@ralph: what do you think about it?

No. We haven't educated/documented how to waive for composes yet, so we can get away without it.

(I only documented how to waive by result_id for bodhi updates and associated koji builds...)

Would be clearer to have two versions of API for this?

POST /v1.0/waivers

Only accept result_id.

POST /v2.0/waivers

Only accept subject and testcase.
And the one day we could just deprecate the first one without touching the second one.

I'll go ahead with this solution.

Ah hmm, I don't think it makes sense to have POST /v2.0/waivers if all the other APIs are still under /v1.0/...

Also it means we would have two separate but similar implementations of the same API.

And I don't see that it really gains us any benefit?

I think POST /v1.0/waivers should just accept both 'result_id' key and the new 'subject' and 'testcase' keys, and we should just keep the code forever.

@dcallagh I talked also with @ralph about this in our today's meeting.
I think we can follow your suggestion. I liked the idea of 2 different versions because it is clean, but I don't like at all that we cannot maintain both the versions (since it is a waste of time) and the old one will become obsolete very soon. So we will just maintain one API that accept both "result_id" and subject/testcase.

So at this point I just have to finish some tests and I'll do another PR with the changes.

Closing this in favor of #120.

Pull-Request has been closed by ralph

2 years ago

Build b6c8a45336b57a45633d4e422da05db470e5c1dd FAILED!
Rebase or make new commits to rebuild.

Build b6c8a45336b57a45633d4e422da05db470e5c1dd FAILED!
Rebase or make new commits to rebuild.