Hi Eduard, The latest REST spec change PR LGTM overall.
I think it does make sense to avoid putting vendor-specific credential properties into the REST spec itself. However, I still have a couple of comments. (reposting from GH) REST Servers have to provide concrete values for vendor-specific credential properties. If there is no effort to standardize that, I think the java client implementation (!) will become the de-facto standard for these properties, which is sub-optimal, IMHO. I'd propose to add a separate spec file, which will be distinct from the REST catalog spec, and will serve exclusively to unify how clients and servers interpret vendor-specific credentials for _known_ use cases. Its purpose will not be to restrict what can be done, but to define a common way of handling existing use cases. WDYT? Thanks, Dmitri. On Thu, Oct 10, 2024 at 5:38 AM Eduard Tudenhöfner <etudenhoef...@apache.org> wrote: > Based on recent discussions the feedback was that we don't want to have > anything storage-specific in the OpenAPI spec (other than documenting the > different storage configurations, which is handled by #10576 > <https://github.com/apache/iceberg/pull/10576>). > Therefore I've updated the PR and made it flexible enough so that we could > still pass different credentials based on a given *prefix* but not need a > new Spec change whenever new credentials are added/changed for a > storage provider. > That should hopefully work for everyone, so please take a look so that we > can do a formal VOTE on these changes. > > It was also brought up that it would be helpful to see how this looks in > the context of refreshing vended credentials. I'll share the proposal for > refreshing vended credentials a bit later today. > > Thanks > Eduard > > On Tue, Sep 24, 2024 at 6:34 PM Dmitri Bourlatchkov > <dmitri.bourlatch...@dremio.com.invalid> wrote: > >> > wrt ISO 8601 timestamps: I'd like to keep things consistent with other >> places in the spec, which are typically defined as millisecond values. >> >> Fair enough. Now that the spec states the reference point in time, using >> millisecond offsets is fine. >> >> Cheers, >> Dmitri. >> >> On Tue, Sep 24, 2024 at 10:41 AM Eduard Tudenhöfner < >> etudenhoef...@apache.org> wrote: >> >>> Thanks Dmitri for reviewing the PR and the doc. >>> >>> wrt ISO 8601 timestamps: I'd like to keep things consistent with other >>> places in the spec, which are typically defined as millisecond values. >>> >>> Thanks >>> Eduard >>> >>> On Fri, Sep 20, 2024 at 4:46 PM Dmitri Bourlatchkov >>> <dmitri.bourlatch...@dremio.com.invalid> wrote: >>> >>>> Thanks for proposing this improvement, Eduard! >>>> >>>> Overall it seems pretty reasonable to me. I added a few comments in GH >>>> and in the doc. >>>> >>>> One higher level point I'd like to discuss is using ISO 8601 to format >>>> expiry timestamps (as opposed to numeric milliseconds values). This should >>>> hopefully make the config more human-readable without adding too much >>>> processing burden. I hope the standard is well supported by most language >>>> libraries now. It is certainly supported by java. WDYT? >>>> >>>> Thanks, >>>> Dmitri. >>>> >>>> On Fri, Sep 13, 2024 at 12:13 PM Eduard Tudenhöfner < >>>> etudenhoef...@apache.org> wrote: >>>> >>>>> Hey everyone, >>>>> >>>>> I'd like to propose standardizing the vended credentials used in >>>>> loadTable / loadView responses. >>>>> >>>>> I opened #11118 <https://github.com/apache/iceberg/issues/11118> to >>>>> track the proposal in GH. >>>>> Please find the proposal doc here >>>>> <https://docs.google.com/document/d/1lySd_5hMZNtISLKsOvAq7xiNzdXU6TAoHF_yrOXWQvM/edit?usp=sharing> >>>>> (estimated >>>>> read time: 5 minutes). >>>>> The proposal requires a spec change, which can be seen in #10722 >>>>> <https://github.com/apache/iceberg/pull/10722>. >>>>> >>>>> Thanks, >>>>> Eduard >>>>> >>>>
