-
Notifications
You must be signed in to change notification settings - Fork 122
Add export ingest-pipelines command #2574
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
base: main
Are you sure you want to change the base?
Conversation
cmd.PersistentFlags().StringP(cobraext.ProfileFlagName, "p", "", fmt.Sprintf(cobraext.ProfileFlagDescription, install.ProfileNameEnvVar)) | ||
|
||
return cobraext.NewCommand(cmd, cobraext.ContextPackage) | ||
} | ||
|
||
func exportDashboardsCmd(cmd *cobra.Command, args []string) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moves export dashboards functionality into own file now that we are adding support for ingest-pipelines
) | ||
|
||
// IngestPipeline contains the information needed to export an ingest pipeline. | ||
type IngestPipeline struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A lot of this functionality was moved to the internal/elasticsearch
package since it shares a lot of retrieval needs with the new export functionality
@@ -0,0 +1,122 @@ | |||
// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file was extracted from cmd/export.go
💚 Build Succeeded
History
|
|
||
ingestPipelineNames = slices.DeleteFunc(ingestPipelineNames, func(name string) bool { | ||
// Filter out system pipelines that start with dot "." or global@ | ||
return strings.HasPrefix(name, ".") || strings.HasPrefix(name, "global@") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any other pipeline types / patterns we should exclude here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is fine by now, we can modify this in the future if there is some feedback from package developers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! This is looking quite good, and works well. Added some comments.
|
||
ingestPipelineNames = slices.DeleteFunc(ingestPipelineNames, func(name string) bool { | ||
// Filter out system pipelines that start with dot "." or global@ | ||
return strings.HasPrefix(name, ".") || strings.HasPrefix(name, "global@") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is fine by now, we can modify this in the future if there is some feedback from package developers.
} | ||
|
||
func promptWriteLocations(pipelineNames []string, writeLocations []export.PipelineWriteLocation) (export.PipelineWriteAssignments, error) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit. Remove empty line.
Description: func(value string, index int) string { | ||
idx := slices.IndexFunc(writeLocations, func(p export.PipelineWriteLocation) bool { return p.Name == value }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't be idx
the same as index
?
@@ -39,18 +43,20 @@ func TestDumpInstalledObjects(t *testing.T) { | |||
&installedObjectsDumpSuite{ | |||
// To reproduce the scenario: | |||
// - Start the stack with version 7.16.2. | |||
// - Install apache package (1.3.4). | |||
// - Install apache package (1.3.5). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit. Why did we need to regenerate the records? Doesn't it work the same for these cases?
yamlBytes, err := yaml.Marshal(jsonPipeline) | ||
if err != nil { | ||
return fmt.Errorf("marshalling ingest pipeline json to yaml failed (ID: %s): %w", pipeline.Name(), err) | ||
} | ||
|
||
// requirement: https://github.com/elastic/package-spec/pull/54 | ||
documentStartDashes := []byte("---\n") | ||
documentBytes := append(documentStartDashes, yamlBytes...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit. To avoid appending, you can use a bytes buffer. Something like this:
yamlBytes, err := yaml.Marshal(jsonPipeline) | |
if err != nil { | |
return fmt.Errorf("marshalling ingest pipeline json to yaml failed (ID: %s): %w", pipeline.Name(), err) | |
} | |
// requirement: https://github.com/elastic/package-spec/pull/54 | |
documentStartDashes := []byte("---\n") | |
documentBytes := append(documentStartDashes, yamlBytes...) | |
var documentBytes bytes.Buffer | |
// requirement: https://github.com/elastic/package-spec/pull/54 | |
documentBytes.WriteString("---\n") | |
err := yaml.NewEncoder(&documentBytes).Encode(jsonPipeline) | |
if err != nil { | |
return fmt.Errorf("marshalling ingest pipeline json to yaml failed (ID: %s): %w", pipeline.Name(), err) | |
} |
|
||
type PipelineWriteAssignments map[string]PipelineWriteLocation | ||
|
||
func IngestPipelines(ctx context.Context, api *elasticsearch.API, writeAssignments PipelineWriteAssignments) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we have a test case for this?
return fmt.Errorf("unmarshalling ingest pipeline failed (ID: %s): %w", pipeline.Name(), err) | ||
} | ||
|
||
yamlBytes, err := yaml.Marshal(jsonPipeline) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should do some formatting here. Some things:
- Remove
_meta
, specially themanaged*
fields should not be needed here. - Remove
version
, I think this is not being used in packages. - Indentation. Looking to pipelines they use to be indented by two spaces, default marshalling seems to use four spaces.
Maybe we should make elastic-package format
to automatically format these files too. But that is another story.
Resolves: #1722
elastic-package export ingest-pipelines