Enhance /tableConfigs/validate endpoint with cluster-aware validations#16675
Open
rseetham wants to merge 1 commit intoapache:masterfrom
Open
Enhance /tableConfigs/validate endpoint with cluster-aware validations#16675rseetham wants to merge 1 commit intoapache:masterfrom
rseetham wants to merge 1 commit intoapache:masterfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #16675 +/- ##
============================================
- Coverage 63.21% 63.20% -0.01%
Complexity 1454 1454
============================================
Files 3179 3179
Lines 191069 191322 +253
Branches 29220 29269 +49
============================================
+ Hits 120786 120934 +148
- Misses 60860 60942 +82
- Partials 9423 9446 +23
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| TableConfigs tableConfigs = tableConfigsAndUnrecognizedProps.getLeft(); | ||
| String databaseName = DatabaseUtils.extractDatabaseFromHttpHeaders(httpHeaders); | ||
| validateConfig(tableConfigs, databaseName, typesToSkip); | ||
| validateConfig(tableConfigs, databaseName, SKIP_CLUSTER_VALIDATIONS); |
Contributor
There was a problem hiding this comment.
If we hardcode this, users will not be able to pass typesToSkip. We should avoid this change or have a logic to add SKIP_CLUSTER_VALIDATIONS into typesToSkip before making the call.
IMO we should keep it as is and let it do validation twice.
noob-se7en
reviewed
Sep 12, 2025
Comment on lines
537
to
539
| Set<TableConfigUtils.ValidationType> skipTypes = typesToSkip == null ? Collections.emptySet() | ||
| : Arrays.stream(typesToSkip.split(",")).map(s -> TableConfigUtils.ValidationType.valueOf(s.toUpperCase())) | ||
| .collect(Collectors.toSet()); |
Contributor
There was a problem hiding this comment.
Pls use the TableConfigsUtil method for this, we can make that public.
| .collect(Collectors.toSet()); | ||
|
|
||
| // Cluster-aware validations | ||
| if (!skipTypes.contains(TableConfigUtils.ValidationType.ALL)) { |
Contributor
There was a problem hiding this comment.
why not add these new validations in TableConfigUtils.validate method itself?
| TableConfigs tableConfigs = tableConfigsAndUnrecognizedProps.getLeft(); | ||
| String databaseName = DatabaseUtils.extractDatabaseFromHttpHeaders(httpHeaders); | ||
| validateConfig(tableConfigs, databaseName, typesToSkip); | ||
| validateConfig(tableConfigs, databaseName, SKIP_CLUSTER_VALIDATIONS); |
| public static final Logger LOGGER = LoggerFactory.getLogger(TableConfigsRestletResource.class); | ||
|
|
||
| // Skip cluster validations during table creation/update since they're performed in the actual table operations | ||
| private static final String SKIP_CLUSTER_VALIDATIONS = "TENANT,MINION_INSTANCES,ACTIVE_TASKS"; |
Contributor
There was a problem hiding this comment.
Pls update API param swagger documentation if including above more types.
rseetham
force-pushed
the
cluster-aware-validation
branch
from
6752e37 to
5c7df7b
Compare
rseetham
force-pushed
the
cluster-aware-validation
branch
2 times, most recently
from
c3006a4 to
0d96668
Compare
rseetham
force-pushed
the
cluster-aware-validation
branch
from
0d96668 to
201ef9a
Compare
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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 enhances the
/tableConfigs/validateendpoint to include cluster-aware validations that were previously only performed during table creation.Currently, some validation errors (like "Failed to find instances with tag: X") only show up in logs during table creation, which makes debugging harder. This change moves those validations to the validation endpoint so you get immediate feedback.
Changes
TENANT,MINION_INSTANCES,ACTIVE_TASKS/tableConfigs/validateto check cluster state by defaultvalidationTypesToSkipparameterThis enables fail-fast validation and better error visibility during development.
Deployed locally and tested