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

Return stats directly from restore function #4713

Merged
merged 1 commit into from
Nov 27, 2023
Merged

Conversation

ashmrtn
Copy link
Contributor

@ashmrtn ashmrtn commented Nov 17, 2023

Rework restore return status so that later PRs will have a smaller diff

Instead of returning a status and then waiting on the message just return the restore stats directly. The status getter was only setup to wait for one item anyway and was setup to wait after the entire restore operation already completed (at end of
m365/restore.go:ConsumeRestoreCollections())


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
  • 🤖 Supportability/Tests
  • 💻 CI/Deployment
  • 🧹 Tech Debt/Cleanup

Issue(s)

Test Plan

  • 💪 Manual
  • ⚡ Unit test
  • 💚 E2E

@ashmrtn ashmrtn added restore tech-debt Non-feature, non-bug improvements to the codebase. labels Nov 17, 2023
@ashmrtn ashmrtn self-assigned this Nov 17, 2023
Copy link
Contributor

aviator-app bot commented Nov 17, 2023

Current Aviator status

Aviator will automatically update this comment as the status of the PR changes.
Comment /aviator refresh to force Aviator to re-examine your PR (or learn about other /aviator commands).

This PR was merged using Aviator.


See the real-time status of this PR on the Aviator webapp.

Use the Aviator Chrome Extension to see the status of your PR within GitHub.

Copy link
Contributor

@ryanfkeepers ryanfkeepers left a comment

Choose a reason for hiding this comment

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

While I'm in favor of the pattern here, the intent is to replace this with the count.Bus anyway. What's your preference: keep this change and undo it later? Or drop it and we'll get around to it with the count.Bus update?

@ashmrtn
Copy link
Contributor Author

ashmrtn commented Nov 18, 2023

Let's keep this for now as it makes switching to service-level handlers much simpler. We can rearrange/remove this after that point

Base automatically changed from 4254-restore-deets to main November 18, 2023 02:23
Instead of returning a status and then waiting on the message just
return the restore stats directly. The status getter was only setup to
wait for one item anyway and was setup to wait after the entire restore
operation already completed (at end of
m365/restore.go:ConsumeRestoreCollections())
Copy link

sonarcloud bot commented Nov 27, 2023

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 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
mergequeue restore tech-debt Non-feature, non-bug improvements to the codebase.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants