Remove FILE from default SUPPORTED_CATALOG_STORAGE_TYPES#1566
Remove FILE from default SUPPORTED_CATALOG_STORAGE_TYPES#1566eric-maynard wants to merge 11 commits intoapache:mainfrom
Conversation
|
#1532 does a lot more than this. See this comment on that PR. |
quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GetConfigTest.java
Show resolved
Hide resolved
dimas-b
left a comment
There was a problem hiding this comment.
This PR makes sense to me end-to-end.
I agree that it conflicts with #1532 is some areas, but at it stands this PR appears to be self-sufficient, consistent and offers net improvement for user experience (removes defaults that users should not use anyway).
I would not mind merging this PR and rebasing #1532 since the latter has a bigger scope of changes and has some comment threads that need to be resolved.
@snazy WDYT?
adnanhemani
left a comment
There was a problem hiding this comment.
Overall, agreed with this change as well. One question - I know we've modified the integration tests, have we confirmed when running them that they continue to work?
|
@snazy can you clarify if there is a technical reason for the -1 here? |
|
Sure, #1532 has to go in first. |
|
And this one duplicates work from #1532 |
|
Why does #1532 have to go in first? It seems like 1532 subsumes this PR but
they should merge quite neatly.
…On Tue, May 20, 2025 at 6:04 PM Robert Stupp ***@***.***> wrote:
*snazy* left a comment (apache/polaris#1566)
<#1566 (comment)>
And this one duplicates work from #1532
<#1532>
—
Reply to this email directly, view it on GitHub
<#1566 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AFRE3SGBTQ7CE5F5R5Q5ACT27NOBZAVCNFSM6AAAAAB43T76IKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQOJVGIYTSNJUGU>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
FILE has been included in the default value for
SUPPORTED_CATALOG_STORAGE_TYPESsince the config's inception, but we should consider removing it as users who don't read throughconfiguring-polaris-for-production.mdmight be inadvertently leaving an insecure storage type enabled.