feat(addon): add instance type #2604
Conversation
📝 WalkthroughWalkthroughThe changes integrate a new field, Changes
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (1.64.8)Error: you are using a configuration file for golangci-lint v2 with golangci-lint v1: please use golangci-lint v2 🔧 SQLFluff (3.3.0)tools/migrate/migrations/20250405193107_addon-instance-type.up.sqlUser Error: No dialect was specified. You must configure a dialect or specify one on the command line using --dialect after the command. Available dialects: tools/migrate/migrations/20250405193107_addon-instance-type.down.sqlUser Error: No dialect was specified. You must configure a dialect or specify one on the command line using --dialect after the command. Available dialects: 📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (18)
🚧 Files skipped from review as they are similar to previous changes (15)
🧰 Additional context used🧬 Code Definitions (3)openmeter/ent/db/addon/addon.go (3)
openmeter/ent/db/addon/where.go (5)
openmeter/ent/db/addon_create.go (3)
⏰ Context from checks skipped due to timeout of 90000ms (9)
🔇 Additional comments (22)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
tothandras
force-pushed
the
feat/addon-type
branch
from
85f8334 to
51a733e
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
openmeter/ent/schema/addon.go (1)
35-36: LGTM! The instance_type field is properly defined.The new field is correctly added to the Addon schema as an Enum type with appropriate Go type mapping.
However, since this is an enum field, consider adding validation to ensure only valid instance types are accepted.
You may want to enhance this implementation by adding validation options:
field.Enum("instance_type"). - GoType(productcatalog.AddonInstanceType("")) + GoType(productcatalog.AddonInstanceType("")). + Comment("The instance type of the addon"). + Validators(validation.NotEmpty)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tools/migrate/migrations/atlas.sumis excluded by!**/*.sum
📒 Files selected for processing (15)
openmeter/ent/db/addon.go(5 hunks)openmeter/ent/db/addon/addon.go(5 hunks)openmeter/ent/db/addon/where.go(2 hunks)openmeter/ent/db/addon_create.go(7 hunks)openmeter/ent/db/addon_update.go(7 hunks)openmeter/ent/db/migrate/schema.go(2 hunks)openmeter/ent/db/mutation.go(8 hunks)openmeter/ent/db/runtime.go(1 hunks)openmeter/ent/schema/addon.go(1 hunks)openmeter/productcatalog/addon.go(4 hunks)openmeter/productcatalog/addon/adapter/addon.go(2 hunks)openmeter/productcatalog/addon/httpdriver/mapping.go(3 hunks)openmeter/productcatalog/addon/service.go(3 hunks)tools/migrate/migrations/20250405192121_addon-instance-type.down.sql(1 hunks)tools/migrate/migrations/20250405192121_addon-instance-type.up.sql(1 hunks)
🧰 Additional context used
🧬 Code Definitions (9)
openmeter/ent/schema/addon.go (1)
openmeter/productcatalog/addon.go (1)
AddonInstanceType(202-202)
openmeter/ent/db/addon.go (2)
openmeter/productcatalog/addon.go (1)
AddonInstanceType(202-202)openmeter/ent/db/addon/addon.go (1)
FieldInstanceType(41-41)
openmeter/ent/db/addon/addon.go (3)
openmeter/productcatalog/addon.go (1)
AddonInstanceType(202-202)openmeter/ent/db/addonratecard/addonratecard.go (1)
OrderOption(143-143)openmeter/ent/db/feature/feature.go (1)
OrderOption(111-111)
openmeter/ent/db/migrate/schema.go (5)
openmeter/ent/db/feature/where.go (1)
Name(89-91)openmeter/ent/db/customer/customer.go (1)
Columns(101-120)openmeter/ent/db/planphase/planphase.go (1)
Columns(62-75)openmeter/ent/db/addon/addon.go (1)
Columns(62-78)openmeter/ent/db/plan/plan.go (1)
Columns(66-81)
openmeter/ent/db/addon_update.go (3)
openmeter/productcatalog/addon.go (1)
AddonInstanceType(202-202)openmeter/ent/db/addon/addon.go (1)
InstanceTypeValidator(116-123)openmeter/ent/db/ent.go (1)
ValidationError(245-248)
openmeter/productcatalog/addon/service.go (1)
openmeter/productcatalog/addon.go (1)
AddonInstanceType(202-202)
openmeter/ent/db/mutation.go (5)
api/api.gen.go (1)
AddonInstanceType(841-841)api/client/go/client.gen.go (1)
AddonInstanceType(774-774)api/client/javascript/src/client/schemas.ts (1)
AddonInstanceType(8822-8822)openmeter/productcatalog/addon.go (1)
AddonInstanceType(202-202)openmeter/ent/db/addon/addon.go (1)
FieldInstanceType(41-41)
openmeter/ent/db/addon_create.go (3)
openmeter/productcatalog/addon.go (1)
AddonInstanceType(202-202)openmeter/ent/db/ent.go (1)
ValidationError(245-248)openmeter/ent/db/addon/addon.go (2)
InstanceTypeValidator(116-123)FieldInstanceType(41-41)
openmeter/ent/db/addon/where.go (6)
openmeter/productcatalog/addon.go (2)
AddonInstanceType(202-202)Addon(160-165)openmeter/ent/db/addon.go (2)
Addon(18-54)Addon(75-94)openmeter/ent/schema/addon.go (5)
Addon(17-19)Addon(21-25)Addon(27-51)Addon(53-60)Addon(62-77)openmeter/ent/db/predicate/predicate.go (1)
Addon(10-10)openmeter/productcatalog/addon/addon.go (1)
Addon(13-21)openmeter/ent/db/addon/addon.go (1)
FieldInstanceType(41-41)
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: Test
- GitHub Check: Quickstart
- GitHub Check: Lint
- GitHub Check: E2E
- GitHub Check: Developer environment
- GitHub Check: CI
- GitHub Check: Commit hooks
- GitHub Check: Build
- GitHub Check: Analyze (go)
🔇 Additional comments (52)
tools/migrate/migrations/20250405192121_addon-instance-type.up.sql (1)
1-2:Details
❓ Verification inconclusive
Verify error handling for existing data during migration.
The migration adds a NOT NULL column without providing a default value. If the table already contains data, this migration will fail unless the column is initially populated.
Consider running the following script to check if the addons table has existing records:
If there are existing records, consider one of these approaches:
- Add a DEFAULT clause to the column definition
- Perform a two-step migration (add nullable column, update data, then add NOT NULL constraint)
🏁 Script executed:
#!/bin/bash # Check if the addons table has existing records that would require a default value psql -c "SELECT COUNT(*) FROM addons;" || echo "Table may not exist yet"Length of output: 192
Attention: Review Migration for Handling Existing Data in Addons Table
The migration in
tools/migrate/migrations/20250405192121_addon-instance-type.up.sqlcurrently adds a NOT NULL column without a default value. This can cause the migration to fail if theaddonstable contains existing data. Our verification script did not conclusively check for existing records because thepsqlcommand was not available, resulting in:
- Output: "Table may not exist yet"
Considerations:
- Ensure that the migration is executed in an environment where
psqlis available and theaddonstable can be reliably queried.- If the table contains records:
- Either add a DEFAULT clause to the column definition.
- Or use a two-step migration: first add the column as nullable, update/populate the column data, and then constrain it as NOT NULL.
tools/migrate/migrations/20250405192121_addon-instance-type.down.sql (1)
1-2: LGTM! The down migration properly reverses the changes.The migration correctly removes the "instance_type" column from the "addons" table.
openmeter/ent/db/runtime.go (1)
89-89: LGTM! Updated index accounts for the new field.This change correctly updates the index from
addonFields[4]toaddonFields[5]to account for the newly added instance_type field, which shifts the position of the annotations field descriptor.openmeter/ent/db/addon.go (5)
14-14: Import dependency added appropriately.The import for
productcatalogis correctly added to support the newAddonInstanceTypefield.
42-43: New InstanceType field looks good.The field is properly defined with the correct type and JSON tag, following the established pattern for other fields in the struct.
83-83: Field correctly added to scanValues method.The
FieldInstanceTypeis properly added to the list of fields that create asql.NullString, which is appropriate sinceAddonInstanceTypeis a string type.
174-179: Proper handling of the new field in assignValues.The case implementation follows the established pattern by checking the type and setting the value with appropriate error handling.
275-277: String representation update is correct.The
Stringmethod is properly updated to include the new field in the string representation.openmeter/ent/db/migrate/schema.go (2)
25-25: New enum column definition is correct.The instance_type column is properly defined as an enum type with valid values "single" and "multiple".
67-67: Index reference properly updated.The column reference in the addon_annotations index is correctly updated to reflect the insertion of the new column.
openmeter/ent/db/addon/addon.go (5)
6-6: Imports added appropriately.The necessary imports for
fmtandproductcatalogare correctly added to support the new functionality.Also applies to: 12-12
40-41: Field constant definition looks good.The constant for the
instance_typefield is properly defined and follows the naming convention.
74-74: Field added to Columns array.The field is correctly added to the Columns array, which is necessary for it to be recognized as a valid column.
115-123: Validator function implementation is correct.The validator function appropriately restricts values to "single" and "multiple" with proper error messaging.
178-181: Ordering function follows established pattern.The
ByInstanceTypefunction correctly implements ordering by the instance_type field, following the same pattern as other ordering functions.openmeter/productcatalog/addon/service.go (3)
140-142: Field added to UpdateAddonInput struct correctly.The InstanceType field is properly defined with the right type and appropriate JSON tag.
172-174: Equal method comparison is implemented correctly.The comparison logic for the new field follows the established pattern, checking only if the field is not nil.
200-204: Validation logic is properly implemented.The validation logic for the InstanceType field correctly handles nil values and uses the field's Validate method.
openmeter/productcatalog/addon/httpdriver/mapping.go (3)
31-31: LGTM: Added proper field mapping for InstanceTypeThe field is properly mapped from the domain type to the API type using a consistent pattern with the existing code.
67-71: LGTM: InstanceType field added to AddonMetaThe InstanceType field is correctly added to the AddonMeta structure in the CreateAddonRequest, following the same pattern as other fields.
96-99: LGTM: InstanceType field included in UpdateAddonRequestThe InstanceType field is properly included in the UpdateAddonRequest structure, correctly wrapped with lo.ToPtr() for nullable handling.
openmeter/productcatalog/addon.go (4)
55-57: LGTM: InstanceType field added to AddonMeta structThe field is properly added with appropriate JSON tag and documentation.
84-86: LGTM: Validation for InstanceType addedThe Validate method is correctly updated to include validation for the InstanceType field, following the same error handling pattern as other fields.
112-114: LGTM: Equal method updated for InstanceTypeThe Equal method is appropriately updated to compare the InstanceType field between two AddonMeta instances.
202-224: LGTM: AddonInstanceType type and methods implementedThe implementation includes:
- A clear type definition as a string
- Well-named constants with descriptive values
- A proper Validate method that checks against valid values
- A Values method that returns all valid string representations
This follows best practices for type-safe enums in Go.
openmeter/ent/db/addon_update.go (6)
126-138: LGTM: SetInstanceType methods added to AddonUpdateBoth the direct setter and nullable setter methods are implemented correctly, following the same pattern as other fields in the struct.
276-280: LGTM: Validation check added for InstanceTypeThe check method is properly updated to validate the InstanceType field using the InstanceTypeValidator.
326-328: LGTM: SQL field setting for InstanceType addedThe sqlSave method correctly sets the InstanceType field in the SQL update specification when it's defined in the mutation.
509-521: LGTM: SetInstanceType methods added to AddonUpdateOneBoth direct and nullable setter methods are implemented correctly, consistent with the AddonUpdate struct implementation.
672-676: LGTM: Validation check added for AddonUpdateOneThe check method in AddonUpdateOne is properly updated to validate the InstanceType field.
739-741: LGTM: SQL field setting for AddonUpdateOneThe sqlSave method for AddonUpdateOne correctly sets the InstanceType field in the SQL update specification.
openmeter/ent/db/addon/where.go (4)
639-643: LGTM: InstanceTypeEQ predicate addedThe equality predicate is correctly implemented for filtering by InstanceType.
645-649: LGTM: InstanceTypeNEQ predicate addedThe non-equality predicate is correctly implemented for filtering by InstanceType.
651-658: LGTM: InstanceTypeIn predicate addedThe inclusion predicate is correctly implemented, with proper type conversion from the domain-specific type to the required internal representation.
660-667: LGTM: InstanceTypeNotIn predicate addedThe non-inclusion predicate is correctly implemented, following the same pattern as InstanceTypeIn.
openmeter/productcatalog/addon/adapter/addon.go (2)
177-177: The instance type field is properly set during addon creation.The addition of
SetInstanceTypeis correctly implemented in theCreateAddonmethod, ensuring that when creating a new addon, the instance type is properly initialized.
438-440: LGTM: Instance type handling added to UpdateAddonThe implementation for updating the instance type follows the same pattern as other nullable fields in the method. The nil check ensures that the instance type is only updated when explicitly provided in the input parameters.
openmeter/ent/db/mutation.go (8)
125-128: Addition of instance_type field follows established patternThe addition of the
instance_typefield to theAddonMutationstruct with the pointer type*productcatalog.AddonInstanceTypeis consistent with the existing field pattern in this struct.
664-698: LGTM: Accessor methods for instance_type are well-implementedAll the accessor methods follow the established patterns in the codebase:
SetInstanceTypesets the valueInstanceTypereturns the current value and existence flagOldInstanceTyperetrieves previous value with proper error handlingResetInstanceTyperesets the fieldThe error handling in
OldInstanceTypethoroughly validates operation type and ID requirements before accessing the database.
935-935: Capacity increased to accommodate new fieldAppropriate increase of the fields slice capacity from 13 to 14 to optimize memory allocation for the new field.
966-968: Field registration follows established patternThe conditional check and field registration for
instance_typein theFields()method follows the consistent pattern used for other fields.
1006-1007: Field accessor properly integratedThe new case for
addon.FieldInstanceTypein theField()method correctly returns the result ofm.InstanceType().
1043-1044: OldField accessor properly integratedThe new case for
addon.FieldInstanceTypein theOldField()method correctly returns the result ofm.OldInstanceType(ctx).
1130-1136: SetField implementation with proper type checkingThe implementation for handling
instance_typein theSetField()method includes appropriate type assertion and error handling, following the pattern used for other fields.
1291-1293: ResetField implementation is correctThe implementation for resetting
instance_typein theResetField()method correctly callsm.ResetInstanceType().openmeter/ent/db/addon_create.go (7)
17-17: Import added correctly for required type.The import for
productcatalogis necessary to use theAddonInstanceTypedefined in that package.
128-132: Setter method is properly implemented.This method follows the consistent pattern used throughout the codebase for field setters - taking a parameter of the correct type, delegating to the mutation object, and returning
*AddonCreatefor method chaining.
293-300: Validation logic added correctly.The validation properly:
- Checks that the required field is present
- Uses the
InstanceTypeValidatorfunction to ensure valid values ("single" or "multiple")- Returns appropriate validation errors with descriptive messages
This maintains consistency with other field validations in the codebase.
380-383: Field creation correctly implemented.The code properly:
- Checks if the instance_type value is present in the mutation
- Sets it in the specification with the correct field type (TypeEnum)
- Sets the value on the node object
This follows the pattern used for other fields in the file.
564-574: Upsert methods properly implemented.These methods provide the ability to:
- Set a specific instance_type value during upsert
- Update instance_type to the value provided during creation
This maintains consistency with other field handling in the upsert operations.
802-814: UpsertOne methods properly implemented.The methods correctly delegate to the AddonUpsert methods through the Update function, maintaining the pattern used for other fields in the AddonUpsertOne structure.
1221-1233: Bulk upsert methods properly implemented.The bulk upsert methods correctly delegate to the AddonUpsert methods through the Update function, maintaining the pattern used for other fields in the AddonUpsertBulk structure.
tothandras
force-pushed
the
feat/addon-type
branch
4 times, most recently
from
de5b011 to
86c8510
Compare
tothandras
force-pushed
the
feat/addon-type
branch
from
86c8510 to
c645523
Compare
2 participants
Summary by CodeRabbit