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

fix: Add filesystem kwargs when read prev_table on FileRetrievalJob (… #3491

Merged
merged 1 commit into from
Mar 2, 2023

Conversation

goodseog
Copy link
Contributor

#3490)

What this PR does / why we need it:
For FileSource ingest to offline store rights.

Which issue(s) this PR fixes:

Fixes #
Use filesystem kwargs for read dataset.
It works with local & s3fs both.

Copy link
Collaborator

@felixwang9817 felixwang9817 left a comment

Choose a reason for hiding this comment

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

/lgtm

@feast-ci-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: felixwang9817, goodseog

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

@felixwang9817
Copy link
Collaborator

@goodseog you'll need to sign your commit!

@feast-ci-bot
Copy link
Collaborator

New changes are detected. LGTM label has been removed.

@goodseog goodseog force-pushed the fix-file-source-read-fs branch 3 times, most recently from 472d163 to 482f6b4 Compare February 13, 2023 15:53
@goodseog
Copy link
Contributor Author

Verified!
Sorry for the frequent push commits due to interworking issues with the company mail gpg key. 😂

@felixwang9817
Copy link
Collaborator

felixwang9817 commented Feb 13, 2023

hmm @goodseog the DCO check is still failing - I don't think you signed off properly?

to be clear, here's what we mean by signing off your commit: https://docs.feast.dev/project/development-guide#signing-off-commits

@goodseog
Copy link
Contributor Author

I hope git commit -s --amend works

@feast-ci-bot
Copy link
Collaborator

New changes are detected. LGTM label has been removed.

@felixwang9817
Copy link
Collaborator

btw @goodseog I think your commit has been signed off properly, but there's some flakiness with our Java tests that's blocking this from merging (I've noticed this in some other PRs as well)

no need for further action on your part, once the Java tests are fixed your PR should land!

@achals achals added the lgtm label Mar 2, 2023
@feast-ci-bot feast-ci-bot merged commit dca4745 into feast-dev:master Mar 2, 2023
kevjumba pushed a commit that referenced this pull request Mar 17, 2023
# [0.30.0](v0.29.0...v0.30.0) (2023-03-17)

### Bug Fixes

* Add description attribute to the Field.from_proto method ([#3469](#3469)) ([473f8d9](473f8d9))
* Add filesystem kwargs when read prev_table on FileRetrievalJob (… ([#3491](#3491)) ([dca4745](dca4745)), closes [#3490](#3490)
* Feature view `entities` from_proto type ([#3524](#3524)) ([57bbb61](57bbb61))
* Fix missing requests requirement after GCP requirement removed. Make BigQuerySource not require gcp extra ([2c85421](2c85421))
* Fix SQL Registry cache miss ([#3482](#3482)) ([3249b97](3249b97))
* Fixed path inside quickstart notebook ([#3456](#3456)) ([66edc32](66edc32))
* Improve BQ point-in-time joining scalability ([#3429](#3429)) ([ff66784](ff66784))
* Pin typeguard to 2.13.3 which is what we are currently using. ([#3542](#3542)) ([61f6fb0](61f6fb0))
* Protobuf lower bound to 3.20 to alert that Feast is incompatible with tensorflow ([#3476](#3476)) ([9ca59e3](9ca59e3))
* Spark kafka processor sorting ([#3479](#3479)) ([f2cbf43](f2cbf43))
* UI working behind base url ([#3514](#3514)) ([9a3fd98](9a3fd98))
* Update go dependencies ([#3512](#3512)) ([bada97c](bada97c))

### Features

* Add Rockset as an OnlineStore ([#3405](#3405)) ([fd91cda](fd91cda))
* Add Snowflake Registry ([#3363](#3363)) ([ec1e61d](ec1e61d))
* Adding query timeout to `to_df` and `to_arrow` retrieval methods ([#3505](#3505)) ([bab6644](bab6644))
* adds k8s config options to Bytewax materialization engine ([#3518](#3518)) ([1883f55](1883f55))
achals pushed a commit that referenced this pull request Mar 24, 2023
# [0.30.0](v0.29.0...v0.30.0) (2023-03-24)

### Bug Fixes

* Add description attribute to the Field.from_proto method ([#3469](#3469)) ([473f8d9](473f8d9))
* Add filesystem kwargs when read prev_table on FileRetrievalJob (… ([#3491](#3491)) ([dca4745](dca4745)), closes [#3490](#3490)
* Bytewax image pull secret config ([#3547](#3547)) ([d2d13b1](d2d13b1))
* Clean up Rockset Online Store for use ([#3549](#3549)) ([a76c6d0](a76c6d0))
* Feature view `entities` from_proto type ([#3524](#3524)) ([57bbb61](57bbb61))
* Fix missing requests requirement after GCP requirement removed. Make BigQuerySource not require gcp extra ([2c85421](2c85421))
* Fix SQL Registry cache miss ([#3482](#3482)) ([3249b97](3249b97))
* Fixed path inside quickstart notebook ([#3456](#3456)) ([66edc32](66edc32))
* Improve BQ point-in-time joining scalability ([#3429](#3429)) ([ff66784](ff66784))
* Pin typeguard to 2.13.3 which is what we are currently using. ([#3542](#3542)) ([61f6fb0](61f6fb0))
* Protobuf lower bound to 3.20 to alert that Feast is incompatible with tensorflow ([#3476](#3476)) ([9ca59e3](9ca59e3))
* Spark kafka processor sorting ([#3479](#3479)) ([f2cbf43](f2cbf43))
* UI working behind base url ([#3514](#3514)) ([9a3fd98](9a3fd98))
* Update go dependencies ([#3512](#3512)) ([bada97c](bada97c))

### Features

* Add Rockset as an OnlineStore ([#3405](#3405)) ([fd91cda](fd91cda))
* Add Snowflake Registry ([#3363](#3363)) ([ec1e61d](ec1e61d))
* Added SnowflakeConnection caching ([#3531](#3531)) ([f9f8df2](f9f8df2))
* Adding query timeout to `to_df` and `to_arrow` retrieval methods ([#3505](#3505)) ([bab6644](bab6644))
* adds k8s config options to Bytewax materialization engine ([#3518](#3518)) ([1883f55](1883f55))
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.

4 participants