"Start Offline-to-online ingestion" method in Python SDK #1051
Conversation
pyalex
requested review from
davidheryanto,
khorshuheng,
woop and
zhilingc
as code owners
feast-ci-bot
added
do-not-merge/work-in-progress
approved
needs-kind
size/XL
size/XXL
and removed
size/XL
labels
There was a problem hiding this comment.
I wonder if there is a way to structure this without the diamond inheritance? After re-reading this a couple times, subjectively i have a pretty hard time following what implements what methods, and what gets called when.
One way I can think of is to maybe split SparkJob interface into two:
SparkJobParamsthat deals with preparing the job to launch (so it provides file path / class name / args)SparkJobthat deals purely with the job lifecycle after it has been launched (so, id/name/status)
pyalex
force-pushed
the
sdk-offline-to-online
branch
from
9f8e086 to
ebf8c6a
Compare
pyalex
force-pushed
the
sdk-offline-to-online
branch
from
ebf8c6a to
42c4480
Compare
|
@oavdeev that makes sense |
Signed-off-by: Oleksii Moskalenko <moskalenko.alexey@gmail.com>
pyalex
force-pushed
the
sdk-offline-to-online
branch
from
aadd53e to
664ab78
Compare
Signed-off-by: Oleksii Moskalenko <moskalenko.alexey@gmail.com>
Signed-off-by: Oleksii Moskalenko <moskalenko.alexey@gmail.com>
Signed-off-by: Oleksii Moskalenko <moskalenko.alexey@gmail.com>
pyalex
changed the title
Signed-off-by: Oleksii Moskalenko <moskalenko.alexey@gmail.com>
| self._entity_source = entity_source | ||
| self._destination = destination | ||
|
|
||
| def get_name(self) -> str: |
There was a problem hiding this comment.
there is a chance that we'll run into limits on name length on different platforms, on EMR it is 256 chars max
| start: datetime, | ||
| end: datetime, | ||
| jar: str, | ||
| **kwargs, |
There was a problem hiding this comment.
doesn't look like we use kwargs
sdk/python/feast/pyspark/launcher.py
Outdated
| @@ -123,22 +129,45 @@ def start_historical_feature_retrieval_job( | |||
| job_id: str, | |||
There was a problem hiding this comment.
i think it would make sense to remove job_id here as well
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: khorshuheng, oavdeev, pyalex 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 |
Co-authored-by: Oleg Avdeev <oleg.v.avdeev@gmail.com> Signed-off-by: Khor Shu Heng <khor.heng@gojek.com>
khorshuheng
force-pushed
the
sdk-offline-to-online
branch
from
77fca22 to
e23ed4f
Compare
Signed-off-by: Khor Shu Heng <khor.heng@gojek.com>
Signed-off-by: Khor Shu Heng <khor.heng@gojek.com>
|
/lgtm |
What this PR does / why we need it:
This PR aim to glue together various backend implementations of job submission and Feast SDK by introducing some cleaner structure with
SparkJob,SparkJobParametersinterfaces and separatelaunchmodules.Also
offline_to_online_ingestionis added toJobLauncherinterface. Implementations for standalone & dataproc launchers provided as well.Which issue(s) this PR fixes:
Fixes #
Does this PR introduce a user-facing change?: