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.

Sign up for GitHub

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

Jump to bottom

Add get_by_id and list_jobs interface to the launcher interface and implement it for EMR #1095

Merged
merged 1 commit into from
Oct 27, 2020
Merged

Add get_by_id and list_jobs interface to the launcher interface and implement it for EMR #1095

merged 1 commit into from
Oct 27, 2020

Conversation

oavdeev
Copy link
Collaborator

@oavdeev oavdeev commented Oct 23, 2020

What this PR does / why we need it:
Job Service will need those to implement its API calls.

Which issue(s) this PR fixes:

Fixes #

Does this PR introduce a user-facing change?:

NONE

oavdeev
oavdeev commented Oct 23, 2020
View reviewed changes
sdk/python/feast/pyspark/launchers/aws/emr_utils.py
state: str
table_name: Optional[str]
output_file_uri: Optional[str]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I know this is a little hacky to have retrieval-job-specific param in the generic JobInfo but i didn't want to build an entire class hierarchy because of this param (at least not yet)

@oavdeev
add get and list jobs interface to launcher and implement it for EMR
612eb93 
Signed-off-by: Oleg Avdeev <oleg.v.avdeev@gmail.com>
@oavdeev
Copy link
Collaborator Author

oavdeev commented Oct 26, 2020

/kind feature

@feast-ci-bot feast-ci-bot added kind/feature New feature or request and removed needs-kind labels Oct 26, 2020
@feast-ci-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: khorshuheng, oavdeev

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

@khorshuheng
Copy link
Collaborator

/lgtm

@feast-ci-bot
Copy link
Collaborator

@oavdeev: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
test-end-to-end-redis-cluster 612eb93 link /test test-end-to-end-redis-cluster

Full PR test history

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.

@khorshuheng khorshuheng merged commit b542df5 into feast-dev:master Oct 27, 2020
@oavdeev oavdeev deleted the get-list-jobs branch October 27, 2020 06:07
@khorshuheng khorshuheng mentioned this pull request Oct 28, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@khorshuheng khorshuheng khorshuheng approved these changes

@davidheryanto davidheryanto Awaiting requested review from davidheryanto

@pyalex pyalex Awaiting requested review from pyalex

@woop woop Awaiting requested review from woop

@zhilingc zhilingc Awaiting requested review from zhilingc

Assignees

@khorshuheng khorshuheng

Labels
approved kind/feature New feature or request lgtm size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@oavdeev @feast-ci-bot @khorshuheng