flyrain commented on PR #1532: URL: https://github.com/apache/polaris/pull/1532#issuecomment-2867243691
I’m strongly against merging either of the two new feature flags introduced: 1. ALLOW_INSECURE_STORAGE_TYPES It's redundant and risky. If we never want FILE/HDFS in production, don’t include them in SUPPORTED_CATALOG_STORAGE_TYPES. That single list is already the gate. Adding another checkbox doesn’t improve safety, it just adds complexity to Helm charts, docs, and support tooling. It's easy to misfire. The flag sits next to real config. If someone copies a dev application.properties to staging, they might enable insecure storage by accident. The existing readiness check already loudly fails if insecure storage is present, we don’t need a second mechanism. It introduces ongoing maintenance burden. Every config flag becomes a permanent obligation: docs, versioning, admin API, test coverage, support. This one delivers no meaningful return. 2. INITIALIZE_DEFAULT_CATALOG_FILEIO_FOR_TEST It belongs in test code, not product config. This is a test scaffold to bootstrap a catalog for Quarkus/Spark ITs. It should live in a @TestProfile, JUnit utility, or -D flag, not the main config namespace. It adds noise to user-facing docs. By putting it in FeatureConfiguration, we surface it in generated reference docs and admin UIs. Operators will ask, “Do I need to set this?” The answer is always no, so let’s not confuse them. Also, it doesn't make sense to me to cleanup them up in followup PRs. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@polaris.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
