Skip to content
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.

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

S3 endpoint configuration #1169

Closed
mike0sv opened this issue Nov 16, 2020 · 4 comments · Fixed by #1172
Closed

S3 endpoint configuration #1169

mike0sv opened this issue Nov 16, 2020 · 4 comments · Fixed by #1172

Comments

@mike0sv
Copy link
Contributor

mike0sv commented Nov 16, 2020

Is your feature request related to a problem? Please describe.
I want to use my on-premise installment of s3 (minio in my case). For this I need a way to provide custom endpoint_url for boto3.client initialization.

Describe the solution you'd like
New configuration option like FEAST_S3_ENDPOINT_URL. And use it's value in feast.staging.storage_client.S3Client.__init__ when creating s3 client. Default None value will ensure backward compatibility.
To configure this option for spark jobs, #1168 can be implemented

@woop
Copy link
Member

woop commented Nov 16, 2020

Makes sense. I think @tsotnet actually thought about adding this support, but we decided not to add it in Feast 0.8.

@mike0sv do you want to take a stab at it?

@mike0sv
Copy link
Contributor Author

mike0sv commented Nov 16, 2020

Yep

mike0sv added a commit to mike0sv/feast that referenced this issue Nov 17, 2020
feast-ci-bot pushed a commit that referenced this issue Nov 22, 2020
* S3 endpoint configuration #1169

Signed-off-by: mike0sv <[email protected]>

* Add allow_no_value=True to ConfigParser

Signed-off-by: mike0sv <[email protected]>

* New constants API
defaults extraction

Signed-off-by: mike0sv <[email protected]>

* fix for other types of get

Signed-off-by: mike0sv <[email protected]>

* return to the old logic and some testing

Signed-off-by: mike0sv <[email protected]>

* oooopsie

Signed-off-by: mike0sv <[email protected]>

* clear

Signed-off-by: mike0sv <[email protected]>
feast-ci-bot pushed a commit that referenced this issue Nov 22, 2020
* S3 endpoint configuration #1169

Signed-off-by: mike0sv <[email protected]>

* Add allow_no_value=True to ConfigParser

Signed-off-by: mike0sv <[email protected]>

* New constants API
defaults extraction

Signed-off-by: mike0sv <[email protected]>

* fix for other types of get

Signed-off-by: mike0sv <[email protected]>

* return to the old logic and some testing

Signed-off-by: mike0sv <[email protected]>

* oooopsie

Signed-off-by: mike0sv <[email protected]>

* remove DEFAULTS logic changes

Signed-off-by: mike0sv <[email protected]>

* reformat

Signed-off-by: mike0sv <[email protected]>

* _upload_to_file_source docs

Signed-off-by: mike0sv <[email protected]>
@woop
Copy link
Member

woop commented Nov 22, 2020

@mike0sv so you have fixed the Python SDK, but how are you going to deal with Spark ingestion jobs that load data from Minio? I think you'll also need to set the endpoint there.

@mike0sv
Copy link
Contributor Author

mike0sv commented Nov 22, 2020

#1168 is about that. it may be reasonable to automatically add S3 endpoint to those additional options, if it was set

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants