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

Modify describe to not list snapshots #376

Merged
merged 5 commits into from
Nov 1, 2019
Merged

Modify describe to not list snapshots #376

merged 5 commits into from
Nov 1, 2019

Conversation

DeepikaDixit
Copy link
Contributor

@DeepikaDixit DeepikaDixit commented Oct 31, 2019

Change Overview

Listing snapshots is an expensive operation. Also, we do not need a list of snapshots.
Changing the command to check for latest instead

Pull request type

Please check the type of change your PR introduces:

  • 🚧 Work in Progress
  • 🌈 Refactoring (no functional changes, no api changes)
  • 🐹 Trivial/Minor
  • 🐛 Bugfix
  • 🌻 Feature
  • 🗺️ Documentation
  • 🤖 Test

Issues

  • #XXX

Test Plan

  • 💪 Manual
  • ⚡ Unit test
  • 💚 E2E

@@ -255,9 +266,7 @@ func resticAzureArgs(profile *param.Profile, repository string) []string {

// GetOrCreateRepository will check if the repository already exists and initialize one if not
func GetOrCreateRepository(cli kubernetes.Interface, namespace, pod, container, artifactPrefix, encryptionKey string, profile *param.Profile) error {
stdout, stderr, err := listSnapshots(profile, artifactPrefix, encryptionKey, cli, namespace, pod, container)
format.Log(pod, container, stdout)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't need to print the output. That will help reduce the long logs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is already printed in getLatestSnapshots

@DeepikaDixit DeepikaDixit changed the title [DRAFT] Modify describe to not list snapshots [K10-2914] Modify describe to not list snapshots Oct 31, 2019
@DeepikaDixit DeepikaDixit marked this pull request as ready for review October 31, 2019 23:35
@DeepikaDixit DeepikaDixit changed the title [K10-2914] Modify describe to not list snapshots Modify describe to not list snapshots Oct 31, 2019
Copy link
Contributor

@pavannd1 pavannd1 left a comment

Choose a reason for hiding this comment

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

+1 LGTM

@DeepikaDixit
Copy link
Contributor Author

Needs +2 @pavannd1 @tdmanv

@mergify mergify bot merged commit 5a12280 into master Nov 1, 2019
@mergify mergify bot deleted the modify-describe branch November 1, 2019 18:32
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

3 participants