cinder returns 201 for override decision

[eviljeff]

Fixes: mozilla/addons#15734

Description

Relaxes the status code enforcement we have for a decision request to Cinder, to only raise for >=400 error codes.

Context

See issue. Cinder originally implemented the override decision endpoint to return a 200, rather than a 201 for the other decision endpoints. Now it's returning a 201.

Testing

It's such a trivial change it doesn't really need testing, but if you want to:

• set your setting CINDER_SERVER_URL to https://mozilla-staging.cinderapp.com/api/v1/
• in reviewer tools force disable a recommended add-on
• in 2nd level approval queue in reviewer tools, requeue the add-on in the reviewer tools
• back in reviewer tools, force disable it again, but this time resolve the (newly created, from requeue) job at the same time
• in 2nd level approval queue in reviewer tools, proceed with the disable
• check in fake mail for the email to the developer about the disable
• check in Cinder that both decisions were recorded - and the second is override of the first

Checklist

• Add #ISSUENUM at the top of your PR to an existing open issue in the mozilla/addons repository.
• Successfully verified the change locally.
• The change is covered by automated tests, or otherwise indicated why doing so is unnecessary/impossible.

[sentry-io]

🔍 Existing Issues For Review

Your pull request is modifying functions with the following pre-existing issues:

📄 File: src/olympia/abuse/cinder.py

Function	Unhandled Issue
create_override_decision	HTTPError: b'{"detail": ["This decision has already been overridden by decision 4d3a8da0-bd71-42f8-98dc-1e29a60a7f2e"]}' ... Event Count: 1

---

_{Did you find this useful? React with a 👍 or 👎}

[KevinMind]

Can we have at least one test returning 200 so that we know our logic doesn't care about the specific code.?

[KevinMind]

I believe this failure is due to the fact that this PR comes from a fork where the ${{ vars.SLACK_ADDONS_PRODUCTION_CHANNEL }} variable is likely not defined on github.

I've already made a patch to fix that but could you define that variable to any non empty string to see if it fixes the issue (proving what I think is wrong is what is wrong)

gh variable set SLACK_ADDONS_PRODUCTION_CHANNEL --body "foo"

should do it. Then rerun the CI.

[eviljeff]

I believe this failure is due to the fact that this PR comes from a fork where the ${{ vars.SLACK_ADDONS_PRODUCTION_CHANNEL }} variable is likely not defined on github.

I've already made a patch to fix that but could you define that variable to any non empty string to see if it fixes the issue (proving what I think is wrong is what is wrong)

gh variable set SLACK_ADDONS_PRODUCTION_CHANNEL --body "foo"

should do it. Then rerun the CI.

I ran gh variable set SLACK_ADDONS_PRODUCTION_CHANNEL --body "foo" locally and reran all the CI and it doesn't seem to have made a difference (the gh docs indicate it's set at a repo level anyway?)

[KevinMind]

They are indeed set on the repo, but in this case there are 2 repos, yours and the origin. It looks like what is happening is that on the pull_request event, github forbids the usage of secrets (we already knew about) but also variables (didn't know).

My patch fixes the issue by making the workflow able to run if the variables are not defined, like in a fork PR.

[KevinMind]

FYI I wouldn't let the bug or my patch delay this patch. As long as all of the other CI is passing, the commit will build just fine on the push since that runs on the origin repo.

[KevinMind]

Make sure to make at least one of the tests return 200 so we know the error handling is enforced to be lazy.

[eviljeff]

Make sure to make at least one of the tests return 200 so we know the error handling is enforced to be lazy.

399f32f did that