Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

feat/enterpriseportal: add instance category#64253

Merged
bobheadxi merged 8 commits into
mainfrom
08-02-feat_enterpriseportal_add_instance_category
Aug 10, 2024
Merged

feat/enterpriseportal: add instance category#64253
bobheadxi merged 8 commits into
mainfrom
08-02-feat_enterpriseportal_add_instance_category

Conversation

@bobheadxi

@bobheadxi bobheadxi commented Aug 2, 2024

Copy link
Copy Markdown
Member

Per some feedback around https://docs.google.com/document/d/1NyBIQvZ-ziNhiz7Yuzbp4f72Gp6EUrVUoBYnnnviXi8/edit, this adds a "instance type" enumeration for each subscription:

  1. PRIMARY: production
  2. SECONDARY: dev/testing/staging/whatever
  3. INTERNAL: for us

Test plan

Updated tests

@bobheadxi bobheadxi requested a review from a team August 2, 2024 19:30
@bobheadxi bobheadxi marked this pull request as ready for review August 2, 2024 19:30
@bobheadxi bobheadxi force-pushed the 08-02-feat_enterpriseportal_add_instance_category branch from 91257ef to 8fb0c02 Compare August 2, 2024 19:32
@bobheadxi bobheadxi force-pushed the 07-19-feat_enterpriseportal_all_subscriptions_apis_use_enterprise_portal_db branch from 0c41032 to 35238a3 Compare August 2, 2024 19:34
@bobheadxi bobheadxi force-pushed the 08-02-feat_enterpriseportal_add_instance_category branch from 8fb0c02 to 25d2927 Compare August 2, 2024 19:35
@bobheadxi bobheadxi force-pushed the 07-19-feat_enterpriseportal_all_subscriptions_apis_use_enterprise_portal_db branch from 35238a3 to 10e500f Compare August 5, 2024 20:08
@bobheadxi bobheadxi force-pushed the 08-02-feat_enterpriseportal_add_instance_category branch from cfa92bb to 794b367 Compare August 5, 2024 20:08
@bobheadxi bobheadxi force-pushed the 07-19-feat_enterpriseportal_all_subscriptions_apis_use_enterprise_portal_db branch from 10e500f to fe448ba Compare August 6, 2024 21:17
@bobheadxi bobheadxi force-pushed the 08-02-feat_enterpriseportal_add_instance_category branch from 67ac762 to d279c46 Compare August 6, 2024 21:17
@bobheadxi bobheadxi force-pushed the 07-19-feat_enterpriseportal_all_subscriptions_apis_use_enterprise_portal_db branch from fe448ba to 94f8044 Compare August 6, 2024 22:42
@bobheadxi bobheadxi force-pushed the 08-02-feat_enterpriseportal_add_instance_category branch from d279c46 to 36692fa Compare August 6, 2024 22:42
@bobheadxi bobheadxi force-pushed the 07-19-feat_enterpriseportal_all_subscriptions_apis_use_enterprise_portal_db branch from 94f8044 to 08922c7 Compare August 6, 2024 22:59
@bobheadxi bobheadxi force-pushed the 08-02-feat_enterpriseportal_add_instance_category branch from 36692fa to 74800c0 Compare August 6, 2024 22:59
@bobheadxi bobheadxi force-pushed the 07-19-feat_enterpriseportal_all_subscriptions_apis_use_enterprise_portal_db branch from 296c4e8 to d1cb91b Compare August 8, 2024 22:10
@bobheadxi bobheadxi force-pushed the 08-02-feat_enterpriseportal_add_instance_category branch from aa686a6 to 94f6079 Compare August 8, 2024 22:10
@bobheadxi bobheadxi force-pushed the 07-19-feat_enterpriseportal_all_subscriptions_apis_use_enterprise_portal_db branch from d1cb91b to 25d4701 Compare August 8, 2024 22:20
@bobheadxi bobheadxi force-pushed the 08-02-feat_enterpriseportal_add_instance_category branch from 94f6079 to 61b395f Compare August 8, 2024 22:21
@bobheadxi bobheadxi force-pushed the 07-19-feat_enterpriseportal_all_subscriptions_apis_use_enterprise_portal_db branch from 25d4701 to d611369 Compare August 8, 2024 22:33
@bobheadxi bobheadxi force-pushed the 08-02-feat_enterpriseportal_add_instance_category branch from 61b395f to 2df8867 Compare August 8, 2024 22:33
@bobheadxi bobheadxi force-pushed the 07-19-feat_enterpriseportal_all_subscriptions_apis_use_enterprise_portal_db branch from d611369 to c13852d Compare August 8, 2024 22:57
@bobheadxi bobheadxi force-pushed the 08-02-feat_enterpriseportal_add_instance_category branch 2 times, most recently from a45bb7c to 21bb9f1 Compare August 8, 2024 23:37
@bobheadxi bobheadxi force-pushed the 07-19-feat_enterpriseportal_all_subscriptions_apis_use_enterprise_portal_db branch from 0d047cc to 4270378 Compare August 8, 2024 23:38
@bobheadxi bobheadxi force-pushed the 08-02-feat_enterpriseportal_add_instance_category branch 2 times, most recently from d19ad55 to f745e78 Compare August 8, 2024 23:51
@bobheadxi bobheadxi force-pushed the 07-19-feat_enterpriseportal_all_subscriptions_apis_use_enterprise_portal_db branch from 8bbd8d5 to ed77a4e Compare August 9, 2024 15:43
@bobheadxi bobheadxi force-pushed the 08-02-feat_enterpriseportal_add_instance_category branch from 350fe64 to ebb0318 Compare August 9, 2024 15:43
@bobheadxi bobheadxi force-pushed the 07-19-feat_enterpriseportal_all_subscriptions_apis_use_enterprise_portal_db branch from ed77a4e to 153c70b Compare August 9, 2024 15:58
Comment thread cmd/enterprise-portal/internal/subscriptionsservice/v1_test.go Outdated
Comment thread cmd/enterprise-portal/internal/subscriptionsservice/adapters_test.go Outdated
Comment on lines 538 to 551

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is it intended that this allows setting the zero value without a force update?
e.g. the following test passes

{
			name: "empty instance_type w/ update_mask",
			update: &subscriptionsv1.UpdateEnterpriseSubscriptionRequest{
				Subscription: &subscriptionsv1.EnterpriseSubscription{
					Id: mockSubscriptionID,
					// All update-able values are zero
				},
				UpdateMask: &fieldmaskpb.FieldMask{Paths: []string{"instance_type"}},
			},
			// All update-able values should be set to their defaults explicitly
			wantUpdateOpts: autogold.Expect(subscriptions.UpsertSubscriptionOptions{
				InstanceType: &sql.NullString{
					String: "ENTERPRISE_SUBSCRIPTION_INSTANCE_TYPE_UNSPECIFIED",
					Valid:  true,
				},
				ForceUpdate: false,
			}),
		},

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

thanks, I added your test case, and updated the implementation such that if the type is zero, we do an InstanceType: &sql.NullString{} (unset as null) update instead

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants