Problem/Motivation
When using generic frontend environment the only allowed response status to consider a deploy triggered is 200.
There are some systems that, on the successful trigger of deploy, return status 201, which seems like a legitimate response because it means "Created".
If 201 is not accepted the new deployment never gets created and we are always stuck with the first deployment. In addition, the content changes never get cleaned, but stack forever as never deployed changes.
Steps to reproduce
Create a generic frontend environment with a hook from a system that returns status 201 on successful trigger of deployment (example: vercel).
Change some content.
Trigger a deployment for this environment.
There is a warning message "created".
The deployment is not processed, there is no new deployment, but the first deployment is still there with all the old changes.
Proposed resolution
Allow response status 201 in the FrontendEnvironmentBase. 201 seems legitimate for the particular task of checking whether the deployment was triggered. This is allowed in build_hooks_bitbucket as well.
Remaining tasks
User interface changes
API changes
Data model changes
| Comment | File | Size | Author |
|---|---|---|---|
| #16 | allow_response_created.patch | 8.65 KB | petar_basic |
| #12 | 3219061-11.patch | 6.45 KB | larowlan |
Comments
Comment #2
petar_basic commentedHere's a patch for the proposed solution.
Comment #3
petar_basic commentedJust changing the name of the patch.
Comment #4
larowlanThanks for this.
Can you add another test case to
\Drupal\Tests\build_hooks\Kernel\BuildTriggerTestfor this.The method
\Drupal\Tests\build_hooks\Kernel\BuildHooksKernelTestBase::assertFrontendEnvironmentBuildHookaccepts aResponseobject, so you should be able to pass a 201.There's an example in
\Drupal\Tests\build_hooks_bitbucket\Kernel\BitbucketBuildHooksTest::testBitbucketFrontendEnvironmentof using a 201Comment #5
larowlanComment #6
petar_basic commentedAdded another test case to
\Drupal\Tests\build_hooks\Kernel\BuildTriggerTestwhich checks if 201 response is fine.Please let me know if there should be any changes.
Comment #7
petar_basic commentedComment #9
petar_basic commentedComment #10
larowlanLooks great, we just need to get it passing.
Comment #12
larowlanAh! there's no schema at all for that plugin
Comment #14
petar_basic commentedI don't know what is wrong with the test now, lets try with expected_url.
Comment #15
petar_basic commentedComment #16
petar_basic commentedOk, figured I need to provide it with dummy url.
Comment #17
petar_basic commentedComment #18
larowlanThanks @petar_basic I'll commit this on Monday, ran out of time today sorry
Comment #20
larowlanComment #21
larowlanWill wait for #3219055: Toolbar shows frontend environments with status 0 before cutting a new release
Comment #22
larowlanWill cut a release because that issue is failing
Comment #23
larowlanThis will go out as 3.2.0
Comment #24
petar_basic commented