Add historical retrieval via job service #1107
Conversation
oavdeev
requested review from
davidheryanto,
khorshuheng,
pyalex,
woop and
zhilingc
as code owners
oavdeev
force-pushed
the
remote-historical-retrieval
branch
from
93bb69e to
cf9e023
Compare
|
/kind feature |
| // Stop a single job | ||
| rpc StopJob (StopJobRequest) returns (StopJobResponse); | ||
| // Cancel a single job | ||
| rpc CancelJob (CancelJobRequest) returns (CancelJobResponse); |
There was a problem hiding this comment.
renamed this for consistency with the existing Job.cancel method
| // the dataframe with retrieved features. | ||
| repeated string file_glob = 1; | ||
| // The Historical Features request that triggered this export job | ||
| GetHistoricalFeaturesRequest request = 2; |
There was a problem hiding this comment.
Removed a bunch of unused (or not yet used) parameters here, we can always add them back later.
| @@ -127,13 +111,13 @@ message StartOfflineToOnlineIngestionJobResponse { | |||
|
|
|||
| message GetHistoricalFeaturesRequest { | |||
| // List of features that are being retrieved | |||
| repeated feast.serving.FeatureReferenceV2 features = 1; | |||
| repeated string feature_refs = 1; | |||
There was a problem hiding this comment.
changed this for consistency with client API
| // Filter jobs by current job status | ||
| JobStatus status = 2; | ||
| } | ||
| bool include_terminated = 1; |
There was a problem hiding this comment.
another change for consistency with client API
| "-d", | ||
| help="Dtypes for entity df, in JSON format", | ||
| required=False, | ||
| ) |
There was a problem hiding this comment.
I don't expect people to use this in production, but it is handy for debugging.
| @@ -116,7 +116,7 @@ class AuthProvider(Enum): | |||
| # Path to certificate(s) to secure connection to Feast Serving | |||
| CONFIG_SERVING_SERVER_SSL_CERT_KEY: "", | |||
| # Default connection timeout to Feast Serving and Feast Core (in seconds) | |||
| CONFIG_GRPC_CONNECTION_TIMEOUT_DEFAULT_KEY: "3", | |||
| CONFIG_GRPC_CONNECTION_TIMEOUT_DEFAULT_KEY: "10", | |||
There was a problem hiding this comment.
Had to bump this since some job service methods in turn make calls to AWS/GCP APIs. Sometimes those take a while especially in development env where your job service is not running in the cloud itself.
| # Enable or disable TLS/SSL to Feast Service | ||
| CONFIG_JOB_SERVICE_ENABLE_SSL_KEY: "False", | ||
| # Path to certificate(s) to secure connection to Feast Job Service | ||
| CONFIG_JOB_SERVICE_SERVER_SSL_CERT_KEY: "", |
There was a problem hiding this comment.
We should prob unify those params with core service in another PR.
|
/kind feature |
|
/retest |
Signed-off-by: Oleg Avdeev <[email protected]>
oavdeev
force-pushed
the
remote-historical-retrieval
branch
from
e7394a3 to
0b4510d
Compare
|
/retest |
Signed-off-by: Oleg Avdeev <[email protected]>
oavdeev
force-pushed
the
remote-historical-retrieval
branch
from
0b4510d to
278f022
Compare
|
/test test-end-to-end |
Signed-off-by: Oleg Avdeev <[email protected]>
oavdeev
force-pushed
the
remote-historical-retrieval
branch
from
278f022 to
68fb26b
Compare
|
@oavdeev: The following test failed, say
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 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 |
|
/lgtm |
What this PR does / why we need it:
This implements end-to-end flow for historical retrieval via job service. Tested only manually for now, until we include job service setup in e2e tests.
Which issue(s) this PR fixes:
Fixes #
Does this PR introduce a user-facing change?: