Refactor customization opt out flag #569
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nelsonkopliku
force-pushed
the
refactor-cutomizable-flag
branch
from
2ce6fa5 to
9d36c87
Compare
nelsonkopliku
force-pushed
the
refactor-cutomizable-flag
branch
from
9d36c87 to
c6e356e
Compare
arbulu89
reviewed
Feb 10, 2025
There was a problem hiding this comment.
Hey @nelsonkopliku
2 things:
- I know I will be annoying with this but, wouldn't be better to use
customization_disabledinstead ofdisable_customization. We are being declarative until now in the dsl anddisable_customizationlooks imperative. I don't know, it looks out of place. Even when we consume the catalog endpoint to get adisable_customizationentry looks strange. @stefanotorresi proposed this value anyway... - Does the
rootentry have precedence over the individual ones? I mean, putting the root value to true makes all the specific values ignored?
EMaksy
approved these changes
Feb 10, 2025
| | `env.architecture_type` | one of `classic`, `angi` | [Architecture types](https://github.com/trento-project/web/blob/main/lib/trento/clusters/enums/hana_architecture_type.ex) | `cluster_type` is one of `hana_scale_up`, `hana_scale_out` | ||
| | `env.ensa_version` | one of `ensa1`, `ensa2`, `mixed_versions` | [ENSA version](https://github.com/trento-project/web/blob/main/lib/trento/clusters/enums/cluster_ensa_version.ex) | `cluster_type` is `ascs_ers` | ||
| | `env.filesystem_type` | one of `resource_managed`, `simple_mount`, `mixed_fs_types` | [Filesystem type](https://github.com/trento-project/web/blob/main/lib/trento/clusters/enums/filesystem_type.ex) | `cluster_type` is `ascs_ers` | ||
| | name | Type | Reference | Applicable | |
There was a problem hiding this comment.
Yeah the formatting is a bit harder not tbh...
|
Hey @arbulu89
Not a problem for me, gonna change.
In the catalog you will get what the specification is in the check, so no interdependencies. |
nelsonkopliku
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR changes the
customizable: true|falseflag in check's spec todisable_customization: true.Note that in the catalog
disable_customizationis being returned as specified in the check (or its default value), while in the group specific api it plays a role in determining whether something iscustomizableor not.