[Do not merge] Add review stacks #71
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Adds a configuration option to Repositories which we can use to determine if, given a webhook event, we should (de)provision stacks when pull requests are opened/closed.
Allow repositories to configure per-Pull-Request auto-provisioning behavior. When "Dynamically provision stacks for Pull Requests" is enabled: The "Allow All" (default) beahavior will provision a stack for every pull request. The "Allow With Label" behavior will provision a stack for a pull request if, and only if, the pull request has a label matching the value provided by the Repository's "Provisioning label" attribute. The "Prevent With Label" behavior will provision stacks for all pull requests EXCEPT those with a label matching the value provided by the Repository's "Provisioning label" attribute.
This allows (efficiently) querying for stacks locked for this reason while retaining a user-friendly presentation.
Permits distinguishing them from manually-provisioned stacks for the purpose of counting and limiting the number which are active at any one time.
…' into pr-review-stacks
When creating a deployment status let's add the `environment_url` to the payload we send to GitHub. The idea is that this gets us whatever awesome features with which the GitHub API uses this value - automatic linking to the deployment environment in the PR. References ---------- - https://developer.github.com/changes/2016-04-06-deployment-and-deployment-status-enhancements/#link-to-a-live-deployment
Give the mounting application a way to define additional links which apply to stacks across the system rather than per-stack.
* Release 0.30.0 * Update changelog with 0.30.0 version entry * Render shipit.yml links as a menu-list in the nav For stacks with more than one or two links the navigation bar starts to wrap links. We _might_ prevent this by rendering the links as a sub-list instead. * Update omniauth-github * Add ability to perform variable substitution to stack links ...when at least one task has been performed on the stack - IE when we have something we can use to grab the environment variables. This maintains the previous functionality of the `shipit.yml` to declare static links for stacks like: ```yml links: logs: https://logs.application.com ``` but also affords consumers the ability to define template strings in the `shipit.yaml` like: ```yml links: logs: https://logs.${ENVIRONMENT}.application.com ``` Which, if the stack's environment is set to `production` will render a link to `https://logs.production.application.com` in the stack's header. Additionally, allow host applications to add environment values to the stack. All stack env values are also exposed to the `Stack#links` method for variable substitution. The host applications can provide their own values to the stack's environment by intercepting messages dispatched to the `Stack#env` - for example, a host application might do something like this in an initializer: config/initializer/shipit.yml ```ruby ActiveSupport::Reloader.to_prepare do ... module EnvironmentExtensions def env super .merge( # ... environment values from host application ) end end require 'shipit/stack' Shipit::Stack.prepend(EnvironmentExtensions) end ``` Co-authored-by: Jean Boussier <[email protected]> Co-authored-by: Jose Luis Salas <[email protected]> Co-authored-by: Jean byroot Boussier <[email protected]>
Require at least one GithubSyncJob has been successful for a stack before refreshing its pull requests. When a pull requests is created before stack sync it will create a detatched Shipit::Commit record for its head ref. When GithubSyncJob runs afterward the Sidekiq/Shipit::GithubSyncJob perpetually emits an ActiveRecord::RecordNotUnique error becuase the GithubSyncJob attempts to create the PullRequst's head ref Shipit::Commit again in a non-detatched state and violates the index_commits_on_sha_and_stack_id uniqueness index.
This prevents us from duplicating data on Github. See Shopify#1042 (comment)
* Adding pre-condition check for uncached deploy spec * Added commit onto DeploySpec stack * Added DeploySpec repo * Added check_deploy_spec stack * Adding test to make sure trigger_continuous_delivery returns when cached_deploy_spec config is empty * removing whitespace * Removed multiline for spec config. Added newline at eof * Removing deletion of canary tasks from deploy spec test * Breaking down trigger_continuous_delivery fn. Removed duplicated env declaration * can_trigger_continuous_delivery * Further refactored trigger_continuous_delivery * Making delivery condition fns private
* Use api responses directly instead of through last_response * Move exception handling * Don't set response if no data * Add mini_racer in development to not depend on node or other system wide JS engine * Preload Stack#repository by default to fix several N+1 * Fix mocha deprecations * Add a power user endpoint to list all stacks of a repo * Implement a stacks#update API * Fix api/stacks#update route * Save the stack before locking if only the spec has changed * Local dev fixes * Add dedicated view for tasks * Add rollback api * Add frozen string literal comment * Make the latest deployed reference branch opt-in Fix: Shopify#955 * Fix timeline page * Abort active deploy on rollback with force * Perform rollback after abort Use mechanism for triggering revert after abort to trigger a rollback to a specific deploy. * Update test to use until commit id rather than deploy * Update npm-lerna.md * Adapt merge status to new GitHub UI Github changed the width of their page which is why the "Add to merge queue" button looks out of place now. I removed the max-width constraint as this gave me the right results when playing around in the chrome web inspector. * Remove capitilisation from ShipIt repo listing * Update dependencies and test Ruby 2.7 * Workaround the active_model_serializer lookup bug Long story short the constant lookip semantic changed slightly in 2.7.0+ and 2.6.6+. Previously `Shipit::Shipit::Commit` would evaluate to `::Shipit:Commit`. Because of this, badly configured AMS would kinda work. Now it need to be more correct. * Update bundler on CI * Release 0.32.0 * rubocop formatting * changed stack query * Fix query and add test for an edge-case. * Disable the Style/PerlBackrefs cop * Avoid loading useless parts of Rails in development * Rename Shipit::PullRequest to Shipit::MergeRequest Implements the proposal of Shopify/shipit-engine issue Shopify#1093 by renaming the concept of the `Shipit::PullRequest` to `Shipit::MergeRequest`. Consideration was given to maintaining the name "PullRequest" where shipit-engine is interacting directly with the Github API - for example, in the `Shipit.github.api client`. In this way the `Shipit::MergeRequest` is related to Github Pull Requests, but not necessarily _of_ a Github Pull Request. The goal is to make way for a more direct modeling of a Github Pull Request in shipit-engine - `Shipit::PullRequest` - which like `Shipit::Commit` models the Github concept. The plan is to use this closer modeling of the Github Pull Request to finish preparations for submission of our "Review Stack" feature to `Shopify/shipit-engine`. Notes ------- This does include non-backward compatible HTTP API changes to shipit-engine - the `pull_requests` collection resources have been renamed to `merge_requests`. References ---------- - Shopify#1093 * Don't redirect on enabling emergency mode * Removes capitalization of environments A-la Shopify#1091 * Serialize rollback once aborted to value * Revert the pull_request -> merge_request renaming in CommitSerializer [ref Shopify#1095] * Merge shopify master (#64) * Don't redirect on enabling emergency mode * Removes capitalization of environments A-la Shopify#1091 * Serialize rollback once aborted to value * Revert the pull_request -> merge_request renaming in CommitSerializer [ref Shopify#1095] Co-authored-by: Jack Li <[email protected]> Co-authored-by: Ben Langfeld <[email protected]> Co-authored-by: Jean byroot Boussier <[email protected]> Co-authored-by: Jean Boussier <[email protected]> Co-authored-by: Jack Li <[email protected]> Co-authored-by: Jean Boussier <[email protected]> Co-authored-by: Darren Worrall <[email protected]> Co-authored-by: Darren Worrall <[email protected]> Co-authored-by: Kaelig Deloumeau-Prigent <[email protected]> Co-authored-by: Robin Brandt <[email protected]> Co-authored-by: Robin Brandt <[email protected]> Co-authored-by: Alex Page <[email protected]> Co-authored-by: Jean byroot Boussier <[email protected]> Co-authored-by: Ruidan Ji <[email protected]> Co-authored-by: Jonathan Geiger <[email protected]> Co-authored-by: rdji123 <[email protected]> Co-authored-by: Ben Langfeld <[email protected]>
benlangfeld
reviewed
Sep 2, 2020
benlangfeld
reviewed
Sep 2, 2020
benlangfeld
reviewed
Sep 2, 2020
benlangfeld
reviewed
Sep 2, 2020
benlangfeld
reviewed
Sep 2, 2020
Co-authored-by: Ben Langfeld <[email protected]>
indiebrain
commented
Sep 3, 2020
Over the course of implementing and evolving the ReviewStack feature of shipit-engine we've evolved how we model the data. This cleans up that data modeling history to present a clear statement of intended changes to the existing schema of shipit-engine. This snapshot of the end product seems more appropriate for inclusion in the upstream PR of ReviewStacks rather than inclusion of every mis-step along the way. This was tested by checking out the shopify/master branch to setup the local database, then checking this branch out and running the migrations. The fact that the schema.rb was only impacted by reverting a change we didn't really want to make to the upstream schema gives me confidence that this chain of migrations is functionally equivalent to what was in place before.
benlangfeld
reviewed
Sep 3, 2020
benlangfeld
approved these changes
Sep 3, 2020
Closed
Last evening we observed a Review Stack was created without a Pull Request. It is our intent that EVERY Review Stack should have an associated Pull Request. We suspect that the call to `PullRequest#update!` failed. Since creating the ReviewStack and PullRequest are flushed to the database independently, we ended up with a ReviewStack that was missing its PullRequest. This change attempts to make the creation of the Review Stack and its Pull Request atomic - ensuring that any Review Stack that is created will have captured its Pull Request.
* No need to path JS String * Fix typos and improve style in repository settings page * Get rid of an unecessary .presence call * Remove MergeRequestAssigment model * Refactor ProvisioningHandler to be less defensive * Lint attr_accessor * Fix duplicate html ID in repo settings.html Co-authored-by: Jean Boussier <[email protected]>
|
Closed since the upstream PR was merged into master here Shopify#1102 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Tracks the proposed upstream changes to "Add Review Stacks." This PR IS NOT intended to be merged into
powerhome/master, rather these changes should come intopowerhome/mastervia a pull / merge fromshopify/masterif/when Shopify#1102 is accepted.Add Review Stacks
This change set represents a months long effort to build, utilize, evolve, and now - hopefully - contribute-for-general-use the concept of "Review Stacks" to shipit-engine.
As discussed in issue Shopify#960, it is our desire to use shipit-engine to provide something akin to Heroku Pipeline Review Applications - a dynamically managed application instance. Review Stacks are intended to run the code of a Github Pull Request as a disposable application instance.
Terminology
Details
Detailed feature documentation, and user guide, are provided in
docs/review_stacks.md.The Review Stack feature's behavior is intended to be flexible and provides options to the host application to:
Anecdotes
We've been using this feature since December 2019 to serve workload across our entire development staff. Here are some metrics to give a sense of how exercised this feature is within our organization. Our host application:
References