#265 Fix posting Bodhi comments
Closed: Fixed None Opened 8 years ago by kparal.

It was decided that we'd like to continue posting Bodhi comments until Bodhi2 learns to inform the maintainers about the results (that is tracked here: https://github.com/fedora-infra/bodhi/issues/117).

Posting Bodhi comments is currently stuck on https://github.com/fedora-infra/bodhi/issues/289 . I'll use this ticket to inform about the progress.

Posting comments to Bodhi itself will probably be fixed soon. What is more problematic, as I've found out, is mocking Bodhi2 in fake_fedorainfra. It now uses FAS OpenID to log in, and I haven't been successful in working around that. I even considered authenticating against FAS with my own FAS credentials, just to make python-fedora happy, and then sending the new comment into fake_fedorainfra the usual way. Unfortunately, I'm getting AuthError: Invalid Request even with my correct credentials, I'm not really sure what's wrong. So overall I've been successful in adjusting fake_fedorainfra for Bodhi2 for read-only query requests, but unsuccessful for read-write comment requests. The question is whether we want to continue to spend time on this, when we know that we want to kill Bodhi comments soon (and then we will not need fake_fedorainfra anymore). I repeat, standard Bodhi comments will work once python-fedora is fixed, the problematic part here is faking the server with fake_fedorainfra. Of course, if anyone wants to dig deep into OpenID auth methods, I'll gladly post my current WIP code.

Thoughts?


This ticket had assigned some Differential requests:
D526
D527
D542

Honestly, I don't see much benefit to pursuing a fix to fake_fedorainfra. Looking into a PR which disables openid auth for the bodhi client seems like a better use of time right now. If we're going to re-enable comments, would testing against bodhi.stg be an option?

We should have fedmsg emission before too long and we've wanted to kill off the bodhi comment code for a while now - I can't justify stubbing out openid for a couple of weeks of notifications after getting almost no notice on this upgrade. In retrospect, expecting them to tell us more than a week in advance when bodhi2 was coming may have been a bit naive - lesson learned.

My vote is for testing against bodhi.stg to bandaid the comment code long enough for fedmsgs to work (if that's an option), killing fake_fedorainfra and preparing the gallows for the bodhi_comment directive.

OK, sounds reasonable.

Testing against bodhi.stg is an option. If we want to have it very manual, we can just edit bodhi_utils.py and set Bodhi2Client(staging=True). It works, I tested it. If we want to make it easier (and use it more often) we will have to remove bodhi_server: url from taskotron.yaml and replace it with something like bodhi_staging: True/False. Or, if we want to have more control, we would need to have bodhi_server: url and bodhi_openid_provider: url. But unless we mock our own openid provider, we would be able to use it just with bodhi.stg url and bodhi.stg openid provider, nothing else, so bodhi_staging: True/False looks simpler.

The thing is that arbitrary Bodhi URLs work only for read-only operations, read-write operations require an openid provider. By default that is id.fp.o, if you create BodhiClient(staging=True), it's id.stg.fp.o. If you set your own bodhi url (e.g. bodhi.stg), you need to set the openid provider manually, it's not automatically switched. So unless we have a big reason for setting our custom Bodhi URLs, I'd go with bodhi_staging: True/False. WDYT?

In the future, if we require testing Bodhi write operations with our custom URLs, I'd rather request such functionality in python-fedora itself, than mocking it complicatedly.

! In #583#7399, @kparal wrote:
The thing is that arbitrary Bodhi URLs work only for read-only operations, read-write operations require an openid provider. By default that is id.fp.o, if you create BodhiClient(staging=True), it's id.stg.fp.o. If you set your own bodhi url (e.g. bodhi.stg), you need to set the openid provider manually, it's not automatically switched. So unless we have a big reason for setting our custom Bodhi URLs, I'd go with bodhi_staging: True/False. WDYT?

I don't like hardcoding urls in our code if we don't have to. I see why bodhi_staging: true/false would help but I'd rather see that done in addition to the bodhi url so that we're not hardcoding the bodhi endpoints in our code.

We wouldn't be hardcoding URLs, we would be using default values provided by python-fedora. Is that also covered in what you said, or is that OK? Since we don't have now any use case for having bodhi_server url configurable anymore, I was thinking about simplifying the code (instead of complicating it). And we can bring the option back easily if we found a use case for it.

In case you still want to keep it, this seems to be the most straightforward approach:
1. make bodhi_server an empty string by default
2. as long as bodhi_server is empty, respect bodhi_staging setting (False by default)
3. if bodhi_server is non-empty, ignore bodhi_staging. Only read operations will work this way, unless we introduce bodhi_openid_provider as well.

But again, I'm not really sure why we would be doing this, when we don't need it.

I didn't realize that the urls were already embedded in the bodhi client. In that case, bodhi_stabing: true/false works

D527 has been pushed. @tflink, what do you see now as the best course of action? We can deploy it to dev, but we won't be able to test posting bodhi comments anyway. Even if we set bodhi_staging=True, most of the updates won't be found in bodhi.stg. So just deploy it to dev, test it for a few days to make sure it's sane, and then deploy to prod, enable bodhi comments and hope I tested it properly?

The changes seem to be deployed. So right now we just need to make sure the emails really arrive and bodhi is not supressing them. I tried to ping some people who should have received them in the last few days. We'll see.

I have created a custom broken update to test this:
https://bodhi.fedoraproject.org/updates/sendKindle-2.1-6.fc21
Now waiting until depcheck flags it as failed.

The emails are being sent successfully. Closing this.

Login to comment on this ticket.

Metadata