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

implement abstractstagingclient for azure blob storage #1218

Merged
merged 5 commits into from
Dec 30, 2020

Conversation

jklegar
Copy link
Collaborator

@jklegar jklegar commented Dec 8, 2020

Signed-off-by: Jacob Klegar jacob@tecton.ai

What this PR does / why we need it: Supports Azure Blob Storage as an object store and batch data source as part of #1181

Which issue(s) this PR fixes:

Fixes #

Does this PR introduce a user-facing change?:

Users can now use Azure blob storage as a staging location and as a batch data source. To configure it, they will need to set the environment variables FEAST_AZURE_BLOB_ACCOUNT_NAME and FEAST_AZURE_BLOB_ACCOUNT_ACCESS_KEY 

@jklegar
Copy link
Collaborator Author

jklegar commented Dec 8, 2020

/kind feature

@feast-ci-bot feast-ci-bot added kind/feature New feature or request and removed needs-kind labels Dec 8, 2020
@feast-ci-bot feast-ci-bot added size/L and removed size/M labels Dec 8, 2020
@@ -76,10 +78,10 @@ def export_source_to_staging_location(
os.path.join(source_uri.netloc, source_uri.path)
)
else:
# gs, s3 file provided as a source.
# gs, s3, azure blob file provided as a source.
assert source_uri.hostname is not None
return get_staging_client(source_uri.scheme).list_files(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

seems like the Config object isn't available to be passed to get_staging_client here, so is there a better way to pass in the required parameters (account name and access key) for the Azure client than requiring them to be passed in in the config?

@jklegar jklegar changed the title WIP: implement abstractstagingclient for azure blob storage implement abstractstagingclient for azure blob storage Dec 8, 2020
@jklegar
Copy link
Collaborator Author

jklegar commented Dec 8, 2020

I've done some basic manual testing, not sure if there's more that should be done outside of end-to-end Azure tests - what is the case for S3/GCS?

AZURE_BLOB_ACCOUNT_NAME: Optional[str] = None

#: Account access key for Azure blob storage_client
AZURE_BLOB_ACCOUNT_ACCESS_KEY: Optional[str] = None
Copy link
Member

Choose a reason for hiding this comment

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

It's not possible for this configuration to be read from environmental variables by the azure client?

@woop
Copy link
Member

woop commented Dec 9, 2020

what is the case for S3/GCS?

they are covered by e2e tests

@jklegar jklegar mentioned this pull request Dec 22, 2020
Signed-off-by: Jacob Klegar <jacob@tecton.ai>
Signed-off-by: Jacob Klegar <jacob@tecton.ai>
Signed-off-by: Jacob Klegar <jacob@tecton.ai>
Signed-off-by: Jacob Klegar <jacob@tecton.ai>
Signed-off-by: Jacob Klegar <jacob@tecton.ai>
@woop
Copy link
Member

woop commented Dec 30, 2020

/lgtm

@feast-ci-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jklegar, woop

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@woop woop merged commit 76f05d1 into feast-dev:master Dec 30, 2020
@jklegar jklegar deleted the jacob_azure_blob branch February 4, 2021 23:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants