-
Notifications
You must be signed in to change notification settings - Fork 4
Extended ds download command with specific data structure and version #116
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,25 +12,31 @@ package ds | |
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
| "log/slog" | ||
|
|
||
| "github.com/snowplow/snowplow-cli/internal/console" | ||
| snplog "github.com/snowplow/snowplow-cli/internal/logging" | ||
| "github.com/snowplow/snowplow-cli/internal/model" | ||
| "github.com/snowplow/snowplow-cli/internal/util" | ||
| "github.com/spf13/cobra" | ||
| ) | ||
|
|
||
| var downloadCmd = &cobra.Command{ | ||
| Use: "download {directory ./data-structures}", | ||
| Short: "Download all data structures from BDP Console", | ||
| Short: "Download data structures from BDP Console", | ||
| Args: cobra.MaximumNArgs(1), | ||
| Long: `Downloads the latest versions of all data structures from BDP Console. | ||
| Long: `Downloads data structures from BDP Console. | ||
|
|
||
| Will retrieve schema contents from your development environment. | ||
| By default, downloads the latest versions of all data structures from your development environment. | ||
| If no directory is provided then defaults to 'data-structures' in the current directory. | ||
|
|
||
| By default, data structures with empty schemaType (legacy format) are skipped. | ||
| Use --include-legacy to include them (they will be set to 'entity' schemaType).`, | ||
| Use --include-legacy to include them (they will be set to 'entity' schemaType). | ||
|
|
||
| You can download specific data structures using --vendor, --name, and --format flags. | ||
| You can also download a specific version using --version flag, or all versions using --all-versions flag. | ||
| Use --env flag to filter deployments by environment (DEV, PROD).`, | ||
| Example: ` $ snowplow-cli ds download | ||
|
|
||
| Download data structures matching com.example/event_name* or com.example.subdomain* | ||
|
|
@@ -40,7 +46,19 @@ Use --include-legacy to include them (they will be set to 'entity' schemaType).` | |
| $ snowplow-cli ds download --output-format json ./my-data-structures | ||
|
|
||
| Include legacy data structures with empty schemaType | ||
| $ snowplow-cli ds download --include-legacy`, | ||
| $ snowplow-cli ds download --include-legacy | ||
|
|
||
| Download a specific data structure | ||
| $ snowplow-cli ds download --vendor com.example --name login_click --format jsonschema | ||
|
|
||
| Download a specific version of a data structure | ||
| $ snowplow-cli ds download --vendor com.example --name login_click --format jsonschema --version 1-0-0 | ||
|
|
||
| Download all versions of a data structure | ||
| $ snowplow-cli ds download --vendor com.example --name login_click --format jsonschema --all-versions | ||
|
|
||
| Download only production deployments | ||
| $ snowplow-cli ds download --vendor com.example --name login_click --format jsonschema --all-versions --env PROD`, | ||
| Run: func(cmd *cobra.Command, args []string) { | ||
| dataStructuresFolder := util.DataStructuresFolder | ||
| if len(args) > 0 { | ||
|
|
@@ -50,6 +68,15 @@ Use --include-legacy to include them (they will be set to 'entity' schemaType).` | |
| match, _ := cmd.Flags().GetStringArray("match") | ||
| includeLegacy, _ := cmd.Flags().GetBool("include-legacy") | ||
| plain, _ := cmd.Flags().GetBool("plain") | ||
|
|
||
| // Flags for specific data structure download | ||
| vendor, _ := cmd.Flags().GetString("vendor") | ||
| name, _ := cmd.Flags().GetString("name") | ||
| formatFlag, _ := cmd.Flags().GetString("format") | ||
| version, _ := cmd.Flags().GetString("version") | ||
| allVersions, _ := cmd.Flags().GetBool("all-versions") | ||
| env, _ := cmd.Flags().GetString("env") | ||
|
|
||
| files := util.Files{DataStructuresLocation: dataStructuresFolder, ExtentionPreference: format} | ||
|
|
||
| apiKeyId, _ := cmd.Flags().GetString("api-key-id") | ||
|
|
@@ -64,12 +91,56 @@ Use --include-legacy to include them (they will be set to 'entity' schemaType).` | |
| snplog.LogFatalMsg("client creation fail", err) | ||
| } | ||
|
|
||
| dss, err := console.GetAllDataStructures(cnx, c, match, includeLegacy) | ||
| if err != nil { | ||
| snplog.LogFatalMsg("data structure fetch failed", err) | ||
| var dss []model.DataStructure | ||
|
|
||
| // Check if we're downloading a specific data structure | ||
| var includeVersions bool | ||
| if vendor != "" && name != "" && formatFlag != "" { | ||
| // Validate mutually exclusive flags | ||
| if version != "" && allVersions { | ||
| snplog.LogFatalMsg("validation error", fmt.Errorf("--version and --all-versions are mutually exclusive")) | ||
| } | ||
|
|
||
|
Comment on lines
+99
to
+103
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can be expressed with cobra's MarkFlagsMutuallyExclusive |
||
| // Generate hash for the specific data structure | ||
| dsHash := console.GenerateDataStructureHash(org, vendor, name, formatFlag) | ||
|
|
||
| if allVersions { | ||
| // Download all versions | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: You could consider removing some of the inline commentsβparticularly around this code block and line 116 and here. The logic is already quite clear and self-explanatory, so the comments may not be necessary. |
||
| dss, err = console.GetAllDataStructureVersions(cnx, c, dsHash, env) | ||
| if err != nil { | ||
| snplog.LogFatalMsg("failed to fetch all data structure versions", err) | ||
| } | ||
| slog.Info("downloaded all versions", "vendor", vendor, "name", name, "count", len(dss), "env", env) | ||
| includeVersions = true | ||
| } else if version != "" { | ||
| // Download specific version | ||
| ds, err := console.GetSpecificDataStructureVersion(cnx, c, dsHash, version) | ||
| if err != nil { | ||
| snplog.LogFatalMsg("failed to fetch specific data structure version", err) | ||
| } | ||
| dss = []model.DataStructure{*ds} | ||
| slog.Info("downloaded specific version", "vendor", vendor, "name", name, "version", version) | ||
| includeVersions = true | ||
| } else { | ||
| // Download latest version | ||
| ds, err := console.GetSpecificDataStructure(cnx, c, dsHash) | ||
| if err != nil { | ||
| snplog.LogFatalMsg("failed to fetch specific data structure", err) | ||
| } | ||
| dss = []model.DataStructure{*ds} | ||
| slog.Info("downloaded specific data structure", "vendor", vendor, "name", name) | ||
| includeVersions = false // Latest version doesn't need version suffix | ||
| } | ||
| } else { | ||
| // Download all data structures | ||
| dss, err = console.GetAllDataStructures(cnx, c, match, includeLegacy) | ||
| if err != nil { | ||
| snplog.LogFatalMsg("data structure fetch failed", err) | ||
| } | ||
| includeVersions = false // Bulk download gets latest versions without version suffix | ||
| } | ||
|
|
||
| err = files.CreateDataStructures(dss, plain) | ||
| err = files.CreateDataStructuresWithVersions(dss, plain, includeVersions) | ||
| if err != nil { | ||
| snplog.LogFatal(err) | ||
| } | ||
|
|
@@ -85,4 +156,12 @@ func init() { | |
| downloadCmd.PersistentFlags().StringArrayP("match", "", []string{}, "Match for specific data structure to download (eg. --match com.example/event_name or --match com.example)") | ||
| downloadCmd.PersistentFlags().Bool("include-legacy", false, "Include legacy data structures with empty schemaType (will be set to 'entity')") | ||
| downloadCmd.PersistentFlags().Bool("plain", false, "Don't include any comments in yaml files") | ||
|
|
||
| // New flags for specific data structure download | ||
| downloadCmd.PersistentFlags().String("vendor", "", "Vendor of the specific data structure to download (requires --name and --format)") | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This should be expressed as MarkFlagsRequiredTogether |
||
| downloadCmd.PersistentFlags().String("name", "", "Name of the specific data structure to download (requires --vendor and --format)") | ||
| downloadCmd.PersistentFlags().String("format", "jsonschema", "Format of the specific data structure to download (requires --vendor and --name)") | ||
| downloadCmd.PersistentFlags().String("version", "", "Specific version of the data structure to download (optional, defaults to latest)") | ||
| downloadCmd.PersistentFlags().Bool("all-versions", false, "Download all versions of the data structure (mutually exclusive with --version)") | ||
| downloadCmd.PersistentFlags().String("env", "", "Filter deployments by environment (DEV, PROD) - only applies to --all-versions") | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,248 @@ | ||
| /* | ||
| Copyright (c) 2013-present Snowplow Analytics Ltd. | ||
| All rights reserved. | ||
| This software is made available by Snowplow Analytics, Ltd., | ||
| under the terms of the Snowplow Limited Use License Agreement, Version 1.0 | ||
| located at https://docs.snowplow.io/limited-use-license-1.0 | ||
| BY INSTALLING, DOWNLOADING, ACCESSING, USING OR DISTRIBUTING ANY PORTION | ||
| OF THE SOFTWARE, YOU AGREE TO THE TERMS OF SUCH LICENSE AGREEMENT. | ||
| */ | ||
|
|
||
| package ds | ||
|
|
||
| import ( | ||
| "os" | ||
| "path/filepath" | ||
| "testing" | ||
| ) | ||
|
|
||
| func TestDownloadCommand_VersionNaming(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| args []string | ||
| expectedFiles []string | ||
| expectedError bool | ||
| description string | ||
| }{ | ||
| { | ||
| name: "specific_version_download", | ||
| args: []string{"--vendor", "com.example", "--name", "test-schema", "--format", "jsonschema", "--version", "1-0-0", "--api-key-id", "test-id", "--api-key", "test-key", "--org-id", "test-org", "--host", "http://test.com"}, | ||
| expectedFiles: []string{"com.example/test-schema_1-0-0.yaml"}, | ||
| expectedError: true, // Will fail due to mock server, but we're testing the logic | ||
| description: "Should create file with version suffix for specific version download", | ||
| }, | ||
| { | ||
| name: "latest_version_download", | ||
| args: []string{"--vendor", "com.example", "--name", "test-schema", "--format", "jsonschema", "--api-key-id", "test-id", "--api-key", "test-key", "--org-id", "test-org", "--host", "http://test.com"}, | ||
| expectedFiles: []string{"com.example/test-schema.yaml"}, | ||
| expectedError: true, // Will fail due to mock server, but we're testing the logic | ||
| description: "Should create file without version suffix for latest version download", | ||
| }, | ||
| { | ||
| name: "all_versions_download", | ||
| args: []string{"--vendor", "com.example", "--name", "test-schema", "--format", "jsonschema", "--all-versions", "--api-key-id", "test-id", "--api-key", "test-key", "--org-id", "test-org", "--host", "http://test.com"}, | ||
| expectedFiles: []string{"com.example/test-schema_1-0-0.yaml", "com.example/test-schema_2-0-0.yaml"}, | ||
| expectedError: true, // Will fail due to mock server, but we're testing the logic | ||
| description: "Should create files with version suffixes for all versions download", | ||
| }, | ||
| { | ||
| name: "bulk_download", | ||
| args: []string{"--api-key-id", "test-id", "--api-key", "test-key", "--org-id", "test-org", "--host", "http://test.com"}, | ||
| expectedFiles: []string{"com.example/test-schema.yaml"}, | ||
| expectedError: true, // Will fail due to mock server, but we're testing the logic | ||
| description: "Should create files without version suffixes for bulk download", | ||
| }, | ||
| { | ||
| name: "mutually_exclusive_flags", | ||
| args: []string{"--vendor", "com.example", "--name", "test-schema", "--format", "jsonschema", "--version", "1-0-0", "--all-versions", "--api-key-id", "test-id", "--api-key", "test-key", "--org-id", "test-org", "--host", "http://test.com"}, | ||
| expectedFiles: []string{}, | ||
| expectedError: true, | ||
| description: "Should fail when both --version and --all-versions are specified", | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| // Create temporary directory for test | ||
| tempDir := t.TempDir() | ||
| args := append(tt.args, tempDir) | ||
|
|
||
| // Create a new command for each test to avoid state pollution | ||
| cmd := downloadCmd | ||
|
|
||
| // Set up the command | ||
| cmd.SetArgs(args) | ||
|
|
||
| // Execute the command | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: same for these comments. thanks! |
||
| err := cmd.Execute() | ||
|
|
||
| if tt.expectedError { | ||
| if err == nil { | ||
| t.Errorf("Expected error for test case '%s', but got none", tt.name) | ||
| } | ||
| // For expected errors, we don't check file creation | ||
| return | ||
| } | ||
|
|
||
| if err != nil { | ||
| t.Errorf("Unexpected error for test case '%s': %v", tt.name, err) | ||
| return | ||
| } | ||
|
|
||
| // Check if expected files were created | ||
| for _, expectedFile := range tt.expectedFiles { | ||
| filePath := filepath.Join(tempDir, expectedFile) | ||
| if _, err := os.Stat(filePath); os.IsNotExist(err) { | ||
| t.Errorf("Expected file %s was not created for test case '%s'", expectedFile, tt.name) | ||
| } | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestDownloadCommand_FlagValidation(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| args []string | ||
| expectError bool | ||
| description string | ||
| }{ | ||
| { | ||
| name: "vendor_without_name_and_format", | ||
| args: []string{"--vendor", "com.example", "--api-key-id", "test-id", "--api-key", "test-key", "--org-id", "test-org", "--host", "http://test.com"}, | ||
| expectError: false, // This should work as it's a bulk download | ||
| description: "Should work as bulk download when only vendor is specified", | ||
| }, | ||
| { | ||
| name: "name_without_vendor_and_format", | ||
| args: []string{"--name", "test-schema", "--api-key-id", "test-id", "--api-key", "test-key", "--org-id", "test-org", "--host", "http://test.com"}, | ||
| expectError: false, // This should work as it's a bulk download | ||
| description: "Should work as bulk download when only name is specified", | ||
| }, | ||
| { | ||
| name: "version_without_specific_ds", | ||
| args: []string{"--version", "1-0-0", "--api-key-id", "test-id", "--api-key", "test-key", "--org-id", "test-org", "--host", "http://test.com"}, | ||
| expectError: false, // This should work as it's a bulk download (version flag is ignored) | ||
| description: "Should work as bulk download when version is specified without vendor/name/format", | ||
| }, | ||
| { | ||
| name: "all_versions_without_specific_ds", | ||
| args: []string{"--all-versions", "--api-key-id", "test-id", "--api-key", "test-key", "--org-id", "test-org", "--host", "http://test.com"}, | ||
| expectError: false, // This should work as it's a bulk download (all-versions flag is ignored) | ||
| description: "Should work as bulk download when all-versions is specified without vendor/name/format", | ||
| }, | ||
| { | ||
| name: "env_without_all_versions", | ||
| args: []string{"--vendor", "com.example", "--name", "test-schema", "--format", "jsonschema", "--env", "PROD", "--api-key-id", "test-id", "--api-key", "test-key", "--org-id", "test-org", "--host", "http://test.com"}, | ||
| expectError: false, // This should work (env flag is ignored for specific version downloads) | ||
| description: "Should work when env is specified without all-versions", | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| // Create temporary directory for test | ||
| tempDir := t.TempDir() | ||
| args := append(tt.args, tempDir) | ||
|
|
||
| // Create a new command for each test to avoid state pollution | ||
| cmd := downloadCmd | ||
|
|
||
| // Set up the command | ||
| cmd.SetArgs(args) | ||
|
|
||
| // Execute the command | ||
| err := cmd.Execute() | ||
|
|
||
| if tt.expectError && err == nil { | ||
| t.Errorf("Expected error for test case '%s', but got none", tt.name) | ||
| } | ||
|
|
||
| if !tt.expectError && err != nil { | ||
| t.Errorf("Unexpected error for test case '%s': %v", tt.name, err) | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestDownloadCommand_IncludeVersionsLogic(t *testing.T) { | ||
| // This test verifies the logic for determining when to include versions in filenames | ||
| // We'll test the logic by checking the command structure and flag combinations | ||
|
|
||
| testCases := []struct { | ||
| name string | ||
| vendor string | ||
| nameFlag string | ||
| format string | ||
| version string | ||
| allVersions bool | ||
| expectedInclude bool | ||
| description string | ||
| }{ | ||
| { | ||
| name: "specific_version", | ||
| vendor: "com.example", | ||
| nameFlag: "test-schema", | ||
| format: "jsonschema", | ||
| version: "1-0-0", | ||
| allVersions: false, | ||
| expectedInclude: true, | ||
| description: "Should include versions for specific version download", | ||
| }, | ||
| { | ||
| name: "all_versions", | ||
| vendor: "com.example", | ||
| nameFlag: "test-schema", | ||
| format: "jsonschema", | ||
| version: "", | ||
| allVersions: true, | ||
| expectedInclude: true, | ||
| description: "Should include versions for all versions download", | ||
| }, | ||
| { | ||
| name: "latest_version", | ||
| vendor: "com.example", | ||
| nameFlag: "test-schema", | ||
| format: "jsonschema", | ||
| version: "", | ||
| allVersions: false, | ||
| expectedInclude: false, | ||
| description: "Should not include versions for latest version download", | ||
| }, | ||
| { | ||
| name: "bulk_download", | ||
| vendor: "", | ||
| nameFlag: "", | ||
| format: "", | ||
| version: "", | ||
| allVersions: false, | ||
| expectedInclude: false, | ||
| description: "Should not include versions for bulk download", | ||
| }, | ||
| } | ||
|
|
||
| for _, tc := range testCases { | ||
| t.Run(tc.name, func(t *testing.T) { | ||
| // Test the logic that determines includeVersions | ||
| var includeVersions bool | ||
|
|
||
| // This mirrors the logic in the download command | ||
| if tc.vendor != "" && tc.nameFlag != "" && tc.format != "" { | ||
| if tc.allVersions { | ||
| includeVersions = true | ||
| } else if tc.version != "" { | ||
| includeVersions = true | ||
| } else { | ||
| includeVersions = false // Latest version doesn't need version suffix | ||
| } | ||
| } else { | ||
| includeVersions = false // Bulk download gets latest versions without version suffix | ||
| } | ||
|
|
||
| if includeVersions != tc.expectedInclude { | ||
| t.Errorf("Test case '%s': expected includeVersions=%v, got %v. %s", | ||
| tc.name, tc.expectedInclude, includeVersions, tc.description) | ||
| } | ||
| }) | ||
| } | ||
| } | ||
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 you please update any references from "BDP" to just "Console"? Weβve recently completed a rename at Company level. Thanks a lot for your help with this!