Hi JB,

The rationale is just internal consistency of data. I believe the fix
mostly safeguards against user mistakes and the change affects only Polaris
Servers (the PR has been merged).

A similar PR was opened for the StorageAccessConfig: [4226]

That case certainly deserves more discussion as it affects Polaris Clients.
The question is: why we have multiple expiration timestamp properties
there? TBH, I do not know the answer. IIRC, that code dates back to the
original code drop, so I hope some of the original code authors might be
able to shed some light on this.

[4226] https://github.com/apache/polaris/pull/4226

Cheers,
Dmitri.

On Thu, Apr 16, 2026 at 2:08 AM Jean-Baptiste Onofré <[email protected]>
wrote:

> Thanks for sharing Dmitri.
>
> Initially I was confused by the two expire-at properties in the PR. I think
> it would be better to use be consistent here with a single one.
> Also, in the PR, it seems we can have multiple keys per expire-at. That's
> not obvious to me :) I would suggest to add a comment to provide
> background/rationale (for maintenance reason in the future).
>
> Thanks!
> Regards
> JB
>
> On Wed, Apr 15, 2026 at 11:18 PM Dmitri Bourlatchkov <[email protected]>
> wrote:
>
> > Hi All,
> >
> > PR [4173] adds extra validation to ConnectionCredentials to avoid
> ambiguity
> > WRT expiration timestamps.
> >
> > I believe this is a solid fix worth merging.
> >
> > However, there is a potential for breakage in environments that
> previously
> > had incorrect configuration.
> >
> > All in all, the PR has been in review for 3 days. I propose merging
> > tomorrow.
> >
> > [4173] https://github.com/apache/polaris/pull/4173
> >
> > Cheers,
> > Dmitri.
> >
>

Reply via email to