Add feature flag to allow maintainers to create MR pipelines async
What does this MR do and why?
This is related to #463355 (closed) as this endpoint often times out. Allowing it to be created asynchronously will mean that we can create pipelines that take longer than 60s to create.
From my local testing this change has minimal UX impact on the merge requests pipeline page. That is because we are already polling for new pipelines constantly so having it created async shouldn't really matter to the user. I even tested the scenario where the pipeline config is broken and both provide a similar UX before and after. The UX differences are:
- The spinner stops immediately because the async request returns immediately and the user has no visual indicator that it's still being created async
- You might have to wait slightly longer for the polling interval to see the new pipeline when you create it async
- If the pipeline has validation errors then the user won't get that feedback at all for async creation
Follow up UX improvements
I would like to introduce the UX improvements in a follow up issue #476627 (closed) because:
- It's behind a user scoped feature flag and I don't intend to roll this out to customers yet
- This provides immediate value to GitLab maintainers that understand the UX tradeoff because at present we often can't even create pipelines. Personally I see over 60% of my pipeline creation fails and I can't do my job
- Making GitLab maintainers more productive ASAP means we'll be able to improve the product faster and fix the underlying causes (e.g. performance) faster
- The async polling for the specific pipeline failure is quite a bit more complicated to implement as we need to store the information about recent pipeline attempts somewhere
There are more ideas in #463355 (closed) about how we might like to stream/poll from some endpoint to know if the specific pipeline I just tried to create failed or not. From a UX perspective this only seems relevant to timeouts today as it seems these are the ones where a user gets a flash notice of the failure. But the async process basically makes the timeouts go away so it is less relevant. In any case this is behind a feature flag and it will make me and other GitLab maintainers much more productive right now as we often cannot even start a pipeline and we're wasting a lot of resources trying and timing out constantly.
In future, if we need, we can update this API to return an identifier related to the async pipeline you created and then we can add a new endpoint to see whether or not your request succeeded. This could be done by streaming or polling and we could either use ActionCable subscriptions to notify the clients or we could store these recent jobs in redis with a short timeout if that's simpler than a whole new GraphQL endpoint.
MR acceptance checklist
Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.
Screenshots or screen recordings
Screenshots are required for UI changes, and strongly recommended for all other merge requests.
Before | After | |
---|---|---|
With good pipeline | ![]() |
![]() |
With broken pipeline config | ![]() |
![]() |
How to set up and validate locally
Numbered steps to set up and validate the change are strongly suggested.
Related to #463355 (closed)
Merge request reports
Activity
changed milestone to %Backlog
assigned to @DylanGriffith
added pipelinetier-1 label
- A deleted user
added backend label
1 Message CHANGELOG missing: If this merge request needs a changelog entry, add the
Changelog
trailer to the commit message you want to add to the changelog.If this merge request doesn't need a CHANGELOG entry, feel free to ignore this message.
Reviewer roulette
Category Reviewer Maintainer backend @paulobarros
(UTC-3, 13 hours behind author)
@hmehra
(UTC+10, same timezone as author)
frontend @agulina
(UTC+2, 8 hours behind author)
@dmishunov
(UTC+2, 8 hours behind author)
Please refer to documentation page for guidance on how you can benefit from the Reviewer Roulette, or use the GitLab Review Workload Dashboard to find other available reviewers.
If needed, you can retry the
danger-review
job that generated this comment.Generated by
Dangeradded 1 commit
- 9c2a0eb3 - Add frontend for async MR pipeline creation with feature flag
- A deleted user
added feature flag feature flagexists frontend labels
- Resolved by Dylan Griffith
requested review from @allison.browne
added 1 commit
- a62deb78 - Add frontend for async MR pipeline creation with feature flag
added 1 commit
- 51bb2add - Add frontend for async MR pipeline creation with feature flag
requested review from @aalakkad
added 1 commit
- 53795d72 - Add frontend for async MR pipeline creation with feature flag
- Resolved by Dylan Griffith
mentioned in issue #463355 (closed)
- Resolved by Dylan Griffith
- Resolved by Dylan Griffith
@DylanGriffith I left 2 non-blocking suggestions for your consideration, I'll let you have a look and assign the maintainer whenever you see fit. Thanks!
removed review request for @aalakkad
added pipeline:mr-approved label
added pipelinetier-2 label and removed pipelinetier-1 label
Before you set this MR to auto-merge
This merge request will progress on pipeline tiers until it reaches the last tier: pipelinetier-3. We will trigger a new pipeline for each transition to a higher tier.
Before you set this MR to auto-merge, please check the following:
- You are the last maintainer of this merge request
- The latest pipeline for this merge request is pipelinetier-3 (You can find which tier it is in the pipeline name)
- This pipeline is recent enough (created in the last 8 hours)
If all the criteria above apply, please set auto-merge for this merge request.
See pipeline tiers and merging a merge request for more details.
E2E Test Result Summary
allure-report-publisher
generated test report!e2e-test-on-gdk:
test report for 629ade10expand test summary
+------------------------------------------------------------------+ | suites summary | +-------------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +-------------+--------+--------+---------+-------+-------+--------+ | Verify | 83 | 0 | 4 | 0 | 87 | ✅ | | Create | 127 | 0 | 12 | 0 | 139 | ✅ | | Data Stores | 31 | 0 | 1 | 0 | 32 | ✅ | | Plan | 70 | 0 | 0 | 0 | 70 | ✅ | | Govern | 71 | 0 | 0 | 0 | 71 | ✅ | | Package | 16 | 0 | 15 | 0 | 31 | ✅ | | Monitor | 8 | 0 | 0 | 0 | 8 | ✅ | | Release | 5 | 0 | 0 | 0 | 5 | ✅ | | Secure | 2 | 0 | 0 | 0 | 2 | ✅ | | Manage | 0 | 0 | 1 | 0 | 1 | ➖ | | Analytics | 2 | 0 | 0 | 0 | 2 | ✅ | | Fulfillment | 1 | 0 | 0 | 0 | 1 | ✅ | +-------------+--------+--------+---------+-------+-------+--------+ | Total | 416 | 0 | 33 | 0 | 449 | ✅ | +-------------+--------+--------+---------+-------+-------+--------+
e2e-package-and-test:
test report for 629ade10expand test summary
+-------------------------------------------------------------+ | suites summary | +--------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +--------+--------+--------+---------+-------+-------+--------+ | Verify | 102 | 0 | 30 | 6 | 132 | ✅ | +--------+--------+--------+---------+-------+-------+--------+ | Total | 102 | 0 | 30 | 6 | 132 | ✅ | +--------+--------+--------+---------+-------+-------+--------+
- Resolved by Dylan Griffith
- Resolved by Dylan Griffith
- Resolved by Dylan Griffith
- Resolved by Dylan Griffith
- Resolved by Dylan Griffith
- Resolved by Allison Browne
It's hard to tell from the quick screenshots, but I think the loading animation and disabling of the button is a UX change. Currently, refreshing the page gets rid of the loading spinner. We might need to get UX approval for that. @DylanGriffith can you clarify if that is a UX change.
@DylanGriffith, I like this approach but my main concern is that there are a few UX changes like with the loading spinner and alerts for non-persisted pipelines.
requested review from @allison.browne
- Resolved by Sunjung Park
Over to @kushalpandya for frontend maintainer review since the first frontend review suggestions were non-blocking and I've applied some changes.
Back to @allison.browne for backend review. I've applied your suggestions.
@sunjungp could you please review for UX. I've noted in the MR description the UX tradeoffs here and also the fact that this is behind a feature flag and for now I want to unblock GitLab maintainers that can't run pipelines but hopefully we can introduce these more complicated UX improvements as a followup but let me know if you have any concerns with that plan.
requested review from @sunjungp and @kushalpandya
mentioned in issue #476627 (closed)
added pipelinetier-3 label and removed pipelinetier-2 label
- Resolved by Furkan Ayhan