Skip to content

Implement JobService API calls & connect it to SDK - #1111

Closed
tsotnet wants to merge 12 commits into
masterfrom
job-service-server
Closed

Implement JobService API calls & connect it to SDK#1111
tsotnet wants to merge 12 commits into
masterfrom
job-service-server

Conversation

@tsotnet

@tsotnet tsotnet commented Oct 29, 2020

Copy link
Copy Markdown
Collaborator

What this PR does / why we need it:

  • Implement start_offline_to_online_ingestion, start_historical_feature_retrieval_job, start_stream_to_online_ingestion SDK calls with JobService (including branching logic depending on whether job service is enabled or not)
  • Set correct environment for JobService (Helm Charts & Dockerfile)
  • Update JobService protos as necessary (according to Slack discussion)

@feast-ci-bot

Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: tsotnet
To complete the pull request process, please assign zhilingc
You can assign the PR to them by writing /assign @zhilingc in a comment when ready.

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

Details 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

Signed-off-by: Tsotne Tabidze <tsotnet@gmail.com>
@tsotnet
tsotnet force-pushed the job-service-server branch from 1ca0a75 to 221badc Compare October 29, 2020 06:01
Comment thread sdk/python/feast/constants.py Outdated
Comment thread infra/docker/jobservice/Dockerfile
Comment thread infra/docker/jobservice/Dockerfile Outdated
Signed-off-by: Tsotne Tabidze <tsotnet@gmail.com>
Signed-off-by: Tsotne Tabidze <tsotnet@gmail.com>
…ne mode

* New API calls (start_offline_to_online_ingestion, start_stream_to_online_ingestion) now return Remote Jobs instead of job ids
* Implement list_jobs & get_job for standalone mode (looks like Spark is running in local mode and we can't get job statuses so we have to keep cache in memory)
* Wire up list_jobs & get_job on client side with job service
* Tested locally on Feast 101 notebook, everything works

Signed-off-by: Tsotne Tabidze <tsotnet@gmail.com>
Signed-off-by: Tsotne Tabidze <tsotnet@gmail.com>
@tsotnet

tsotnet commented Oct 30, 2020

Copy link
Copy Markdown
Collaborator Author

/test python-sdk-integration-test

Comment thread sdk/python/feast/pyspark/launchers/standalone/local.py Outdated
Signed-off-by: Tsotne Tabidze <tsotnet@gmail.com>
Signed-off-by: Tsotne Tabidze <tsotnet@gmail.com>
Signed-off-by: Tsotne Tabidze <tsotnet@gmail.com>
@tsotnet tsotnet changed the title Implement half of JobService functionality Implement JobService API calls & connect it to SDK Nov 2, 2020
Comment thread sdk/python/feast/remote_job.py
Comment thread sdk/python/feast/pyspark/launchers/standalone/local.py Outdated
Signed-off-by: Tsotne Tabidze <tsotnet@gmail.com>
Signed-off-by: Tsotne Tabidze <tsotnet@gmail.com>
@tsotnet

tsotnet commented Nov 3, 2020

Copy link
Copy Markdown
Collaborator Author

/test test-end-to-end

…mode

Signed-off-by: Tsotne Tabidze <tsotnet@gmail.com>
@tsotnet
tsotnet force-pushed the job-service-server branch from 1c89ea2 to 3335ae4 Compare November 3, 2020 11:02
@tsotnet

tsotnet commented Nov 3, 2020

Copy link
Copy Markdown
Collaborator Author

/test test-end-to-end-aws

Signed-off-by: Tsotne Tabidze <tsotnet@gmail.com>
feature_table = self.client.get_feature_table(
request.table_name, request.project
)
job = self.client.start_offline_to_online_ingestion(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

can we use launcher.start_offline_to_online instead of client here?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

the reason is: client may have different (user-oriented) API (like accepting strings instead of feature table, dataframes instead of source) and it's not really idempotent (and being already called once)

@tsotnet tsotnet Nov 4, 2020

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done in the fork: #1129. Please comment there if anything else is needed.

@tsotnet

tsotnet commented Nov 4, 2020

Copy link
Copy Markdown
Collaborator Author

Closing this and instead opening a PR in a fork: #1129

@tsotnet tsotnet closed this Nov 4, 2020
@tsotnet
tsotnet deleted the job-service-server branch November 4, 2020 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/feature New feature or request size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants