Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add workspace experiment to maintain state between updates and capture success/failure of each #6693

Merged
merged 10 commits into from Jun 20, 2023

Conversation

bdragon
Copy link
Member

@bdragon bdragon commented Feb 17, 2023

Over the course of a version update, Dependabot will typically use the in_a_temporary_repo_directory and in_a_temporary_directory shared helpers (depending on whether the current ecosystem has been configured to always_clone_for_package_manager? or not, respectively) to make changes to manifest files using the native package manager.

This PR introduces the concept of a shared, or persistent workspace as an experiment behind the shared_workspace feature flag. When the experiment is enabled, it will only be used by always-clone package managers (in practice: go_modules, npm_and_yarn, pub, and terraform) or when vendoring dependencies. This is because the implementation is backed by git and requires a git repo to work. In the future, we may simply git init (like this) before using the in_a_temporary_directory shared helper so that the concept can be used across all ecosystems regardless of whether or not they clone.

A workspace is simply a git repo: successful changes are commits and failed changes are stashes. When the experiment is enabled and in_a_temporary_repo_directory is invoked, its block argument will be passed to the workspace's #change method. If the block exits without failure, any changes to the repo contents will be captured as a successful change and committed to the repo; if the block raises, the state of the repo directory (and the exception that was raised) will be captured as a failed change attempt and stashed to the repo.

This concept allows us to iteratively make changes to manifest files, each change starting from the state of the previous successful change, rather than starting from scratch each time. This has obvious potential benefits for grouped updates, in which several updates are made in succession over the course of a single version update job. As of this PR, if a change raises, the exception will be re-raised to mirror current behavior. In the future we may want to inspect and potentially suppress the failure and carry on with the next update in order to allow partially-successful grouped updates.

While this experiment may ultimately support grouped updates, it is being released separately so that we can start evaluating its viability independent of grouped updates work.

Copy link
Contributor

@brrygrdn brrygrdn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! I really like the workspace.change {} call signature, and I can see the methods you've created being something we can plug into logging to better surface errors.

common/spec/dependabot/workspace_spec.rb Outdated Show resolved Hide resolved
@bdragon bdragon force-pushed the bdragon/shared-workspace branch 3 times, most recently from 136c1d9 to 0c2a4f4 Compare February 28, 2023 21:00
@bdragon bdragon force-pushed the bdragon/shared-workspace branch 5 times, most recently from 581606c to a1e6de5 Compare March 3, 2023 18:46
@bdragon bdragon force-pushed the bdragon/shared-workspace branch 4 times, most recently from 3ed4c41 to 4976fae Compare March 17, 2023 20:35
@bdragon bdragon force-pushed the bdragon/shared-workspace branch 14 times, most recently from bd64f0b to 56069fb Compare March 24, 2023 17:16
@bdragon bdragon force-pushed the bdragon/shared-workspace branch 2 times, most recently from 8aaca08 to 1059a9c Compare March 30, 2023 18:17
@bdragon bdragon force-pushed the bdragon/shared-workspace branch 2 times, most recently from 55f34c5 to 5d47c09 Compare March 31, 2023 22:02
@bdragon bdragon marked this pull request as ready for review March 31, 2023 22:28
@bdragon bdragon requested a review from a team as a code owner March 31, 2023 22:28
@bdragon bdragon changed the title POC: stateful git-based shared workspace Add workspace experiment to maintain state between updates and capture success/failure of each Mar 31, 2023
Copy link
Contributor

@brrygrdn brrygrdn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have one question about the options hash as I think it is unused for now so it might make sense to just omit it for now and bring it back once we have proven things out more.

common/lib/dependabot/shared_helpers.rb Outdated Show resolved Hide resolved
common/lib/dependabot/shared_helpers.rb Outdated Show resolved Hide resolved
Copy link
Contributor

@brrygrdn brrygrdn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me - as I mentioned in a comment above I think we should reduce the scope of this PR to just introducing the workspace library code and then add in the call sites downstream of the changes in #7404.

That'll allow us to ship this without impacting any production jobs and have a smoke test in place for vendoring that proves this and #7404 fix the problem before we ship to production.

common/lib/dependabot/shared_helpers.rb Show resolved Hide resolved
updater/lib/dependabot/updater.rb Outdated Show resolved Hide resolved
dependabot-ci added 9 commits June 20, 2023 16:10
Modifies `Dependabot::SharedHelpers.in_a_temporary_repo_directory` so that if the `:shared_workspace` experiment is enabled, a `Dependabot::Workspace::Git` will be initialized and used to execute the given block, unless an existing workspace is given via the `:workspace` kwarg, in which case the given block will be executed using the existing workspace.
Build a workspace for the in-flight job and set Dependabot::Workspace.active_workspace if the :shared_workspace experiment is enabled.
 accepts an optional memo to be associated with each change attempt. We were looking for it in the kwargs given to  but in order to avoid changes to that interface we will omit the memo for now.
Keeping the `Dependabot::Workspace` library code but holding off on integrating it until vendoring support for grouped updates is complete.
Copy link
Contributor

@brrygrdn brrygrdn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the updates, lets :shipit:

@bdragon bdragon merged commit d7fa9fb into main Jun 20, 2023
83 checks passed
@bdragon bdragon deleted the bdragon/shared-workspace branch June 20, 2023 20:57
brettfo pushed a commit to brettfo/dependabot-core that referenced this pull request Oct 11, 2023
…kspace

Add workspace experiment to maintain state between updates and capture success/failure of each
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants