Hi All, I opened [3071] based on this discussion. Please review.
[3071] https://github.com/apache/polaris/pull/3071 Thanks, Dmitri. On 2025/11/14 21:52:28 Adnan Hemani wrote: > Hi all, > > I'm okay with removing the aws-cn block given that it passes, at a minimum, > the unit tests with reasonable values for the AWS CN partition (please > note, the unit tests as they are written now look to be wrong) - and then > awaiting user feedback. > > Best, > Adnan Hemani > > On Mon, Nov 10, 2025 at 5:54 PM Dmitri Bourlatchkov > <[email protected]> wrote: > > > Hi Eric, > > > > The code change here is removing a very specific check [1] that prevents > > using aws-cn. Why would expanding the scope of allowed AWS parameters > > require new tests? We already have plenty of tests that cover AWS SDK > > usage. > > > > As to aws-cn specifically, it does not look like there is any rationale for > > blocking other than general compatibility concerns. I propose to delegate > > the latter to prospective users of aws-cn ARNs. > > > > We could, of course, wait for users to request aws-cn to be supported, but > > given previous PRs that expand the ARN regex in other areas of the > > codebase, I do not see why Polaris should keep blocking aws-cn. > > > > [1] > > > > https://github.com/apache/polaris/blob/034959e23059e19da455320eabc34af6814b1e43/polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsStorageConfigurationInfo.java#L165 > > > > Cheers > > Dmitri. > > > > On Mon, Nov 10, 2025 at 8:44 PM Eric Maynard <[email protected]> > > wrote: > > > > > Just to be really clear, the hypothetical [code] change we’re discussing > > > would take the form of a PR. I don’t think it’s really unreasonable to > > ask > > > that a PR include tests for the functionality being added? It seems like > > > that’s basically why the functionality was left off in the cited PR. > > Sounds > > > like nobody is opposed to adding it, but surely it’s a good idea to test > > > changes before making them? > > > > > > —EM > > > > > > On Mon, Nov 10, 2025 at 11:58 AM Dmitri Bourlatchkov > > > <[email protected]> wrote: > > > > > > > I agree with Robert. I do not think Polaris as a project should test > > all > > > > possible storage scenarios. > > > > > > > > At the same time, Polaris does not appear to have a reason to block > > > aws-cn. > > > > > > > > I propose to remove the explicit block and address user feedback as it > > > > comes in (if it comes in). > > > > > > > > WDYT? > > > > > > > > Thanks, > > > > Dmitri. > > > > > > > > On Mon, Nov 10, 2025 at 5:24 AM Robert Stupp <[email protected]> wrote: > > > > > > > > > Testing every possible ARN rather manually is quite an effort, both > > > > > for the quite special cn and us-gov ones and also for > > appliance/vendor > > > > > specific ones. > > > > > While certain Polaris installations may have a need to forbid those, > > > > > other user-scenarios have legit reasons to allow them. > > > > > I'm not sure whether the Polaris project should enforce anything > > there. > > > > > > > > > > On Sat, Nov 8, 2025 at 1:25 AM Eric Maynard < > > [email protected]> > > > > > wrote: > > > > > > > > > > > > Originally there was no way to specify region/endpoint on a > > catalog, > > > so > > > > > > these regions had to be disabled for the reason given in #1056. I’m > > > > > > guessing nobody has tested aws-cn and so nobody has enabled it. If > > > you > > > > > can > > > > > > test it and it works, I’m definitely in favor of enabling it. > > > > > > > > > > > > —EM > > > > > > > > > > > > > > > > > > On Fri, Nov 7, 2025 at 6:57 PM Dmitri Bourlatchkov < > > [email protected] > > > > > > > > > wrote: > > > > > > > > > > > > > Hi All, > > > > > > > > > > > > > > PR [3005] expanded the RegEx rule for Role ARN parameter > > > validation. > > > > > > > > > > > > > > However, I see [1] that aws-cn ARNs are blocked by an explicit > > code > > > > > check. > > > > > > > This blocking appears to be present since day 1 of the Apache > > > Polaris > > > > > > > codebase [2], when aws-us-gov was also blocked. The blocking of > > > > > aws-us-gov > > > > > > > ended with [1056]. > > > > > > > > > > > > > > Does anyone have any rationale on why Polaris should block aws-cn > > > > ARNs? > > > > > > > > > > > > > > [1] > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://github.com/apache/polaris/blob/main/polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsStorageConfigurationInfo.java#L165 > > > > > > > > > > > > > > [2] > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://github.com/apache/polaris/blob/f3d9141c9708940523aa8d206a0bb32465398a7f/polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsStorageConfigurationInfo.java#L91 > > > > > > > > > > > > > > [1056] https://github.com/apache/polaris/pull/1056 > > > > > > > [3005] https://github.com/apache/polaris/pull/3005 > > > > > > > > > > > > > > Thanks, > > > > > > > Dmitri. > > > > > > > > > > > > > > > > > > > > > >
