Skip to content

Spec: Add expiry time config to REST table load #10873

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from
Closed

Spec: Add expiry time config to REST table load #10873

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e075805
Spec: Add expiry time config to REST table load
munendrasn Aug 4, 2024
f85e695
fix build failure
munendrasn Aug 4, 2024
858e4a0
move the config property to general configuration
munendrasn Aug 5, 2024
b83ee8b
Update open-api/rest-catalog-open-api.yaml
munendrasn Oct 10, 2024
4379856
Update rest-catalog-open-api.py
munendrasn Oct 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions open-api/rest-catalog-open-api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1103,6 +1103,7 @@ class LoadTableResult(BaseModel):
## General Configurations

- `token`: Authorization bearer token to use for table requests if OAuth2 security is enabled
- `expires-at-ms`: if present, specifies timestamp in milliseconds when credentials for storage(AWS S3/Azure/GCP), specified in config would expire
Copy link
Contributor

@nastra nastra Oct 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's actually correct to have this at this level. This should be specific to the underlying Storage provider. GCS for example has gcs.oauth2.token-expires-at. That being said, the respective FileIO properties of each storage provider should have a similar property to the one used for GCS

Copy link
Contributor Author

@munendrasn munendrasn Oct 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on #10722 (comment), I was thinking to move/add this to fileIo Properties once linked PRs in the description are merged
I saw this comment, where it was decided not having the separate property.
It would be beneficial to have this property but will yet to go through the recording of Catalog sync (recording is not yet available)

Copy link
Contributor

@nastra nastra Oct 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the idea is to move the expiration setting to the respective FileIO implementation rather than having a separate field in the OpenAPI spec


## AWS Configurations

Expand Down
3 changes: 2 additions & 1 deletion open-api/rest-catalog-open-api.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2763,7 +2763,8 @@ components:

## General Configurations

- `token`: Authorization bearer token to use for table requests if OAuth2 security is enabled
- `token`: Authorization bearer token to use for table requests if OAuth2 security is enabled
- `expires-at-ms`: if present, specifies timestamp in milliseconds when credentials for storage(AWS S3/Azure/GCP), specified in config would expire

## AWS Configurations

Expand Down