Repository: apigee/registry
Pull request #710 (2022-08-30 23:47:52)

Support max-actions flag in resolve
The default value is `100` for now, it is subject to change once we do some testing while deploying it to the data plane.

codecov[bot]: # [Codecov](https://codecov.io/gh/apigee/registry/pull/710?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee) Report
> Merging [#710](https://codecov.io/gh/apigee/registry/pull/710?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee) (486e83a) into [main](https://codecov.io/gh/apigee/registry/commit/e4ecf46f266bb79484241fa64fefcd0987827644?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee) (e4ecf46) will **increase** coverage by `0.05%`.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff             @@
##             main     #710      +/-   ##
==========================================
+ Coverage   55.60%   55.66%   +0.05%     
==========================================
  Files          93       93              
  Lines        7952     7962      +10     
==========================================
+ Hits         4422     4432      +10     
  Misses       3065     3065              
  Partials      465      465              
```


| [Impacted Files](https://codecov.io/gh/apigee/registry/pull/710?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee) | Coverage Δ | |
|---|---|---|
| [cmd/registry/cmd/resolve/resolve.go](https://codecov.io/gh/apigee/registry/pull/710/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee#diff-Y21kL3JlZ2lzdHJ5L2NtZC9yZXNvbHZlL3Jlc29sdmUuZ28=) | `71.79% <100.00%> (+0.74%)` | :arrow_up: |
| [cmd/registry/controller/controller.go](https://codecov.io/gh/apigee/registry/pull/710/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee#diff-Y21kL3JlZ2lzdHJ5L2NvbnRyb2xsZXIvY29udHJvbGxlci5nbw==) | `76.25% <100.00%> (+0.81%)` | :arrow_up: |

:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee)


Patch:
diff --git a/cmd/registry/cmd/resolve/resolve.go b/cmd/registry/cmd/resolve/resolve.go
index ffd370e..392a483 100644
--- a/cmd/registry/cmd/resolve/resolve.go
+++ b/cmd/registry/cmd/resolve/resolve.go
@@ -55,6 +55,7 @@ func fetchManifest(
 func Command() *cobra.Command {
 	var dryRun bool
 	var jobs int
+	var maxActions int
 	cmd := &cobra.Command{
 		Use:   "resolve MANIFEST_RESOURCE",
 		Short: "resolve the dependencies and update the registry state (experimental)",
@@ -85,7 +86,7 @@ func Command() *cobra.Command {
 			client := &controller.RegistryLister{RegistryClient: registryClient}
 
 			log.Debug(ctx, "Generating the list of actions...")
-			actions := controller.ProcessManifest(ctx, client, name.ProjectID(), manifest)
+			actions := controller.ProcessManifest(ctx, client, name.ProjectID(), manifest, maxActions)
 
 			// The monitoring metrics/dashboards are built on top of the format of the log messages here.
 			// Check the metric filters before making any changes to the format.
@@ -109,9 +110,9 @@ func Command() *cobra.Command {
 			taskQueue, wait := core.WorkerPoolWithWarnings(ctx, jobs)
 			defer wait()
 			// Submit tasks to taskQueue
-			for _, a := range actions {
+			for i := 0; i < len(actions) && i < maxActions; i++ {
 				taskQueue <- &controller.ExecCommandTask{
-					Action: a,
+					Action: actions[i],
 					TaskID: fmt.Sprintf("%.8s", uuid.New()),
 				}
 			}
@@ -120,5 +121,6 @@ func Command() *cobra.Command {
 
 	cmd.Flags().BoolVar(&dryRun, "dry-run", false, "if set, actions will only be printed and not executed")
 	cmd.Flags().IntVarP(&jobs, "jobs", "j", 10, "Number of actions to execute simultaneously")
+	cmd.Flags().IntVarP(&maxActions, "max-actions", "a", 100, "Maximum number of actions to execute")
 	return cmd
 }
diff --git a/cmd/registry/controller/controller.go b/cmd/registry/controller/controller.go
index 95efc27..add76f9 100644
--- a/cmd/registry/controller/controller.go
+++ b/cmd/registry/controller/controller.go
@@ -35,7 +35,8 @@ func ProcessManifest(
 	ctx context.Context,
 	client listingClient,
 	projectID string,
-	manifest *rpc.Manifest) []*Action {
+	manifest *rpc.Manifest,
+	maxActions int) []*Action {
 	var actions []*Action
 	//Check for errors in manifest
 	errs := ValidateManifest(fmt.Sprintf("projects/%s/locations/global", projectID), manifest)
@@ -60,9 +61,19 @@ func ProcessManifest(
 			continue
 		}
 		actions = append(actions, newActions...)
+
+		if len(actions) >= maxActions {
+			log.FromContext(ctx).Debugf("Reached max actions limit %d", maxActions)
+			break
+		}
 	}
 
-	return actions
+	maxLength := len(actions)
+	if maxLength > maxActions {
+		maxLength = maxActions
+	}
+
+	return actions[:maxLength]
 }
 
 func processManifestResource(


Test patch:
diff --git a/cmd/registry/controller/controller-timestamp_test.go b/cmd/registry/controller/controller-timestamp_test.go
index 89242a6..50b9dfb 100644
--- a/cmd/registry/controller/controller-timestamp_test.go
+++ b/cmd/registry/controller/controller-timestamp_test.go
@@ -330,7 +330,7 @@ func TestTimestampArtifacts(t *testing.T) {
 					},
 				},
 			}
-			actions := ProcessManifest(ctx, client, projectID, manifest)
+			actions := ProcessManifest(ctx, client, projectID, manifest, 10)
 
 			if diff := cmp.Diff(test.want, actions, sortActions); diff != "" {
 				t.Errorf("ProcessManifest(%+v) returned unexpected diff (-want +got):\n%s", manifest, diff)
@@ -473,7 +473,7 @@ func TestTimestampAggregateArtifacts(t *testing.T) {
 					},
 				},
 			}
-			actions := ProcessManifest(ctx, client, projectID, manifest)
+			actions := ProcessManifest(ctx, client, projectID, manifest, 10)
 
 			if diff := cmp.Diff(test.want, actions, sortActions); diff != "" {
 				t.Errorf("ProcessManifest(%+v) returned unexpected diff (-want +got):\n%s", manifest, diff)
@@ -655,7 +655,7 @@ func TestTimestampDerivedArtifacts(t *testing.T) {
 					},
 				},
 			}
-			actions := ProcessManifest(ctx, client, projectID, manifest)
+			actions := ProcessManifest(ctx, client, projectID, manifest, 10)
 
 			if diff := cmp.Diff(test.want, actions, sortActions); diff != "" {
 				t.Errorf("ProcessManifest(%+v) returned unexpected diff (-want +got):\n%s", manifest, diff)
@@ -825,7 +825,7 @@ func TestRefreshArtifacts(t *testing.T) {
 					},
 				},
 			}
-			actions := ProcessManifest(ctx, client, projectID, manifest)
+			actions := ProcessManifest(ctx, client, projectID, manifest, 10)
 
 			if diff := cmp.Diff(test.want, actions, sortActions); diff != "" {
 				t.Errorf("ProcessManifest(%+v) returned unexpected diff (-want +got):\n%s", manifest, diff)
diff --git a/cmd/registry/controller/controller_test.go b/cmd/registry/controller/controller_test.go
index ad28c68..2572020 100644
--- a/cmd/registry/controller/controller_test.go
+++ b/cmd/registry/controller/controller_test.go
@@ -163,7 +163,7 @@ func TestArtifacts(t *testing.T) {
 					},
 				},
 			}
-			actions := ProcessManifest(ctx, lister, projectID, manifest)
+			actions := ProcessManifest(ctx, lister, projectID, manifest, 10)
 
 			if diff := cmp.Diff(test.want, actions, sortActions); diff != "" {
 				t.Errorf("ProcessManifest(%+v) returned unexpected diff (-want +got):\n%s", manifest, diff)
@@ -260,7 +260,7 @@ func TestAggregateArtifacts(t *testing.T) {
 					},
 				},
 			}
-			actions := ProcessManifest(ctx, lister, projectID, manifest)
+			actions := ProcessManifest(ctx, lister, projectID, manifest, 10)
 
 			if diff := cmp.Diff(test.want, actions, sortActions); diff != "" {
 				t.Errorf("ProcessManifest(%+v) returned unexpected diff (-want +got):\n%s", manifest, diff)
@@ -402,7 +402,7 @@ func TestDerivedArtifacts(t *testing.T) {
 					},
 				},
 			}
-			actions := ProcessManifest(ctx, lister, projectID, manifest)
+			actions := ProcessManifest(ctx, lister, projectID, manifest, 10)
 
 			if diff := cmp.Diff(test.want, actions, sortActions); diff != "" {
 				t.Errorf("ProcessManifest(%+v) returned unexpected diff (-want +got):\n%s", manifest, diff)
@@ -495,7 +495,7 @@ func TestReceiptArtifacts(t *testing.T) {
 					},
 				},
 			}
-			actions := ProcessManifest(ctx, lister, projectID, manifest)
+			actions := ProcessManifest(ctx, lister, projectID, manifest, 10)
 
 			if diff := cmp.Diff(test.want, actions, sortActions); diff != "" {
 				t.Errorf("ProcessManifest(%+v) returned unexpected diff (-want +got):\n%s", manifest, diff)
@@ -603,7 +603,7 @@ func TestReceiptAggArtifacts(t *testing.T) {
 					},
 				},
 			}
-			actions := ProcessManifest(ctx, lister, projectID, manifest)
+			actions := ProcessManifest(ctx, lister, projectID, manifest, 10)
 
 			if diff := cmp.Diff(test.want, actions, sortActions); diff != "" {
 				t.Errorf("ProcessManifest(%+v) returned unexpected diff (-want +got):\n%s", manifest, diff)
@@ -772,7 +772,7 @@ func TestMultipleEntitiesArtifacts(t *testing.T) {
 					},
 				},
 			}
-			actions := ProcessManifest(ctx, lister, projectID, manifest)
+			actions := ProcessManifest(ctx, lister, projectID, manifest, 10)
 
 			if diff := cmp.Diff(test.want, actions, sortActions); diff != "" {
 				t.Errorf("ProcessManifest(%+v) returned unexpected diff (-want +got):\n%s", manifest, diff)
@@ -780,3 +780,92 @@ func TestMultipleEntitiesArtifacts(t *testing.T) {
 		})
 	}
 }
+
+func TestMaxActions(t *testing.T) {
+	tests := []struct {
+		desc       string
+		seed       []seeder.RegistryResource
+		maxActions int
+	}{
+		{
+			desc: "generated more than maxActions",
+			seed: []seeder.RegistryResource{
+				&rpc.ApiSpec{
+					Name: "projects/controller-test/locations/global/apis/petstore/versions/1.0.0/specs/openapi.yaml",
+				},
+				&rpc.ApiSpec{
+					Name: "projects/controller-test/locations/global/apis/petstore/versions/1.0.1/specs/openapi.yaml",
+				},
+				&rpc.ApiSpec{
+					Name: "projects/controller-test/locations/global/apis/petstore/versions/1.1.0/specs/openapi.yaml",
+				},
+			},
+			maxActions: 2,
+		},
+		{
+			desc: "generated less than maxActions",
+			seed: []seeder.RegistryResource{
+				&rpc.ApiSpec{
+					Name: "projects/controller-test/locations/global/apis/petstore/versions/1.0.0/specs/openapi.yaml",
+				},
+				&rpc.ApiSpec{
+					Name: "projects/controller-test/locations/global/apis/petstore/versions/1.0.1/specs/openapi.yaml",
+				},
+				&rpc.ApiSpec{
+					Name: "projects/controller-test/locations/global/apis/petstore/versions/1.1.0/specs/openapi.yaml",
+				},
+			},
+			maxActions: 4,
+		},
+	}
+
+	const projectID = "controller-test"
+	for _, test := range tests {
+		t.Run(test.desc, func(t *testing.T) {
+			ctx := context.Background()
+			registryClient, err := connection.NewRegistryClient(ctx)
+			if err != nil {
+				t.Fatalf("Failed to create client: %+v", err)
+			}
+			t.Cleanup(func() { registryClient.Close() })
+
+			adminClient, err := connection.NewAdminClient(ctx)
+			if err != nil {
+				t.Fatalf("Failed to create client: %+v", err)
+			}
+			t.Cleanup(func() { adminClient.Close() })
+
+			deleteProject(ctx, adminClient, t, "controller-test")
+			t.Cleanup(func() { deleteProject(ctx, adminClient, t, "controller-test") })
+
+			client := seeder.Client{
+				RegistryClient: registryClient,
+				AdminClient:    adminClient,
+			}
+			lister := &RegistryLister{RegistryClient: registryClient}
+
+			if err := seeder.SeedRegistry(ctx, client, test.seed...); err != nil {
+				t.Fatalf("Setup: failed to seed registry: %s", err)
+			}
+
+			manifest := &rpc.Manifest{
+				Id: "controller-test",
+				GeneratedResources: []*rpc.GeneratedResource{
+					{
+						Pattern: "apis/-/versions/-/specs/-/artifacts/vocabulary",
+						Dependencies: []*rpc.Dependency{
+							{
+								Pattern: "$resource.spec",
+							},
+						},
+						Action: "registry compute vocabulary $resource.spec",
+					},
+				},
+			}
+			actions := ProcessManifest(ctx, lister, projectID, manifest, test.maxActions)
+			if len(actions) > test.maxActions {
+				t.Errorf("ProcessManifest(%+v) generated unexpected number of actions, wanted <= %d, got %d", manifest, test.maxActions, len(actions))
+			}
+		})
+	}
+}