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

SE-100: don't dump when instance is not found #173

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Conversation

pooknull
Copy link
Collaborator

@pooknull pooknull commented Aug 2, 2024

https://perconadev.atlassian.net/browse/SE-100

In this PR, pmm-dump will fail during export when it can't find any data, and it will display the message Failed to create a dump. No data was found.

If the --instance flag was provided, it will issue a warning before failing, with the message It seems that data about instances specified in the `--instance` option does not exist in the PMM server.

@pooknull pooknull marked this pull request as ready for review September 24, 2024 00:54
Copy link
Contributor

@artemgavrilov artemgavrilov left a comment

Choose a reason for hiding this comment

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

LGTM with comments

@@ -44,17 +45,18 @@ func TestExportImport(t *testing.T) {
if err := g.Wait(); err != nil {
t.Fatal(err)
}
time.Sleep(30 * time.Second)
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be great to have comment there what we are waiting here.

Comment on lines +59 to +62
if strings.Contains(stderr, "Failed to create a dump. No data was found") {
// If pmm-dump returns this error, it also means that the dashboard selector parsing was successful
return
}
Copy link
Contributor

@artemgavrilov artemgavrilov Jan 31, 2025

Choose a reason for hiding this comment

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

It looks like relying on exit code may be a better option rather than error text.

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.

3 participants