Skip to content
This repository has been archived by the owner on Oct 11, 2024. It is now read-only.

Wire details merging during BackupOp.Run #1905

Merged
merged 4 commits into from
Dec 23, 2022
Merged

Wire details merging during BackupOp.Run #1905

merged 4 commits into from
Dec 23, 2022

Conversation

ashmrtn
Copy link
Contributor

@ashmrtn ashmrtn commented Dec 21, 2022 •
edited
Loading

Description

Actually call the details merge function while performing a backup. If no base snapshots or len(toMerge) == 0 then turns into a noop.

This PR contains a little extra refactoring to keep from initializing a struct multiple times. However, long-term we should probably refactor BackupOp to better fit the fact that we are now using interfaces as inputs to many functions for the sake of testing

Does this PR need a docs update or release note?

  • ✅ Yes, it's included
  • 🕐 Yes, but in a later PR
  • ⛔ No

Type of change

  • 🌻 Feature
  • 🐛 Bugfix
  • 🗺️ Documentation
  • 🤖 Test
  • 💻 CI/Deployment
  • 🐹 Trivial/Minor

Issue(s)

Test Plan

  • 💪 Manual
  • ⚡ Unit test
  • 💚 E2E

@ashmrtn ashmrtn added the enhancement New feature or request label Dec 21, 2022
@ashmrtn ashmrtn self-assigned this Dec 21, 2022
@ashmrtn ashmrtn temporarily deployed to Testing December 21, 2022 23:07 — with GitHub Actions Inactive
@ashmrtn ashmrtn temporarily deployed to Testing December 21, 2022 23:07 — with GitHub Actions Inactive
@ashmrtn ashmrtn temporarily deployed to Testing December 21, 2022 23:08 — with GitHub Actions Inactive
@ashmrtn ashmrtn temporarily deployed to Testing December 21, 2022 23:08 — with GitHub Actions Inactive
@ashmrtn ashmrtn temporarily deployed to Testing December 21, 2022 23:08 — with GitHub Actions Inactive
Base automatically changed from 1800-merge-func to main December 23, 2022 01:18
@ashmrtn
Minor cleanup: factor out another interface
dddf324 
Pull out another interface and pass in the interface to a function so
that we don't recreate a struct multiple times.

Long-term, we should refactor/consolidate some struct
initialization/location for the backup model.
@ashmrtn
Actually wire up and call mergeDetails in backup
e6d43d9
@ashmrtn
Thread details.Bulider through stack
6e3b4ed 
Now that we're merging details in BackupOp we need to have access to
details.Builder in BackOp.
@ashmrtn ashmrtn force-pushed the 1800-wire-details-merge branch from b699158 to 6e3b4ed Compare December 23, 2022 18:26
@ashmrtn ashmrtn temporarily deployed to Testing December 23, 2022 18:26 — with GitHub Actions Inactive
@ashmrtn ashmrtn temporarily deployed to Testing December 23, 2022 18:26 — with GitHub Actions Inactive
@ashmrtn ashmrtn temporarily deployed to Testing December 23, 2022 18:26 — with GitHub Actions Inactive
@ashmrtn ashmrtn temporarily deployed to Testing December 23, 2022 18:27 — with GitHub Actions Inactive
@ashmrtn ashmrtn temporarily deployed to Testing December 23, 2022 18:27 — with GitHub Actions Inactive
@ashmrtn
Fix rebase error
0ed8566 
Pass base snapshots to hierarhcy merging function.
@ashmrtn ashmrtn temporarily deployed to Testing December 23, 2022 18:41 — with GitHub Actions Inactive
@ashmrtn ashmrtn temporarily deployed to Testing December 23, 2022 18:41 — with GitHub Actions Inactive
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@ashmrtn ashmrtn temporarily deployed to Testing December 23, 2022 18:41 — with GitHub Actions Inactive
@aviator-app
Copy link
Contributor

aviator-app bot commented Dec 23, 2022 •
edited
Loading

Aviator status

Aviator will automatically update this comment as the status of the PR changes.

This PR was merged using Aviator.

@ashmrtn ashmrtn temporarily deployed to Testing December 23, 2022 18:41 — with GitHub Actions Inactive
@ashmrtn ashmrtn temporarily deployed to Testing December 23, 2022 18:41 — with GitHub Actions Inactive
@aviator-app aviator-app bot added the blocked Upstream item prevents completion label Dec 23, 2022
@aviator-app
Copy link
Contributor

aviator-app bot commented Dec 23, 2022

PR failed to merge with reason: some CI status(es) failed.
Failed CI(s): Test-Suite-Trusted

@ashmrtn ashmrtn temporarily deployed to Testing December 23, 2022 18:56 — with GitHub Actions Inactive
@ashmrtn ashmrtn temporarily deployed to Testing December 23, 2022 18:56 — with GitHub Actions Inactive
@ashmrtn ashmrtn temporarily deployed to Testing December 23, 2022 18:56 — with GitHub Actions Inactive
@ashmrtn ashmrtn temporarily deployed to Testing December 23, 2022 18:56 — with GitHub Actions Inactive
@ashmrtn ashmrtn temporarily deployed to Testing December 23, 2022 18:56 — with GitHub Actions Inactive
@ashmrtn ashmrtn temporarily deployed to Testing December 23, 2022 18:56 — with GitHub Actions Inactive
@ashmrtn ashmrtn removed the blocked Upstream item prevents completion label Dec 23, 2022
@aviator-app aviator-app bot merged commit 35c1b10 into main Dec 23, 2022
@aviator-app aviator-app bot deleted the 1800-wire-details-merge branch December 23, 2022 19:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ryanfkeepers ryanfkeepers ryanfkeepers approved these changes

@vkamra vkamra Awaiting requested review from vkamra

Assignees

@ashmrtn ashmrtn

Labels
enhancement New feature or request mergequeue-priority
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

merge backup details from base snapshot(s) and items in Collections
2 participants
@ashmrtn @ryanfkeepers