Skip to content

[CORE] Refactor configuration API to allow explicit getAllEntries calls on specific configuration registry objects#10835

Merged
zhztheplayer merged 5 commits intoapache:mainfrom
zhztheplayer:wip-fix-conf
Oct 3, 2025
Merged

[CORE] Refactor configuration API to allow explicit getAllEntries calls on specific configuration registry objects#10835
zhztheplayer merged 5 commits intoapache:mainfrom
zhztheplayer:wip-fix-conf

Conversation

@zhztheplayer
Copy link
Member

This is to propose a new API to support the calls like:

val allGlutenCoreEntries = GlutenCoreConfig.allEntries
val allGlutenEntries = GlutenConfig.allEntries
val allVeloxEntries = VeloxConfig.allEntries

The story behind this is, I was trying to add a new VeloxDeltaConfig object in PR #10801, but ConfigEntry.getAllEntries returns uncertain results when switching on / off -P delta Maven profile, so CI test that does golden checks for the documented configuration options fails. So we should add a reliable API that returns whatever we need no matter how we trigger the test using Maven.

For example, after the PR change, VeloxConfig.allEntries will return exactly the config options that are registered in VeloxConfig only.

fixup

[VL] Clear configuration entry list before running the golden checks to ensure determinism
@github-actions github-actions bot added CORE works for Gluten Core VELOX CLICKHOUSE DOCS labels Oct 2, 2025
@github-actions
Copy link

github-actions bot commented Oct 2, 2025

Run Gluten Clickhouse CI on x86

@zhztheplayer
Copy link
Member Author

@zjuwangg @yikf Would you help review this? Thanks.

@github-actions
Copy link

github-actions bot commented Oct 2, 2025

Run Gluten Clickhouse CI on x86

@github-actions
Copy link

github-actions bot commented Oct 2, 2025

Run Gluten Clickhouse CI on x86

@github-actions
Copy link

github-actions bot commented Oct 2, 2025

Run Gluten Clickhouse CI on x86

Copy link
Member

@zhouyuan zhouyuan left a comment

Choose a reason for hiding this comment

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

👍

@zhztheplayer
Copy link
Member Author

Thank you @zhouyuan

@zhztheplayer zhztheplayer merged commit 52464e8 into apache:main Oct 3, 2025
99 of 101 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants