-
Notifications
You must be signed in to change notification settings - Fork 310
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
External Plugin Service (grpc) #1524
Changes from 54 commits
2437e6a
063dd3b
cd14658
93a4ed2
a2a5305
f6b0d81
84ffbfe
b39fe48
8ba641c
bda6432
609f852
c06662d
625548b
c446900
787031e
996552c
ce60f20
18af9ac
047b7f1
cf9cf3e
82c048f
19c198c
d66c3f8
5f0084f
2edc620
fd2b9b3
e71b6f6
2b76331
2f598b2
c342d37
ee4a180
1a908c4
764c0f5
f402d57
bc30f51
1c16952
f044e28
8385e02
1dd716d
59714f9
26eab42
4ee1417
a70c12e
dbd26b5
2c1cce8
5a2bdc4
1fba9d4
ae8c37e
0b151cf
9f8337d
4b92275
f28183e
e07c72d
9cadb80
0357806
f594df9
c524560
f11dd2b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
FROM python:3.9-slim-buster | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we build one for each python version? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can use a default version first because it's experimental feature. we can add more version when we need it |
||
|
||
MAINTAINER Flyte Team <[email protected]> | ||
LABEL org.opencontainers.image.source=https://github.com/flyteorg/flytekit | ||
|
||
ARG VERSION | ||
RUN pip install -U flytekit==$VERSION \ | ||
flytekitplugins-bigquery==$VERSION \ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is bigquery special? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. because I add a new backend plugin for BQ in this pr |
||
|
||
CMD pyflyte serve --port 80 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
from concurrent import futures | ||
|
||
import click | ||
import grpc | ||
from flyteidl.service.external_plugin_service_pb2_grpc import add_ExternalPluginServiceServicer_to_server | ||
|
||
from flytekit.extend.backend.external_plugin_service import BackendPluginServer | ||
|
||
_serve_help = """Start a grpc server for the external plugin service.""" | ||
|
||
|
||
@click.command("serve", help=_serve_help) | ||
@click.option( | ||
"--port", | ||
default="80", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this a bit too over-reaching? should we do 30087? 30090? the existing ports are https://github.com/flyteorg/flytectl/blob/bd6b85605f49ff4877484edd69942ca72f4e19dd/pkg/docker/docker_util.go#L135 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. admin also uses 80, maybe set nodeport to 30090 flyteadmin ClusterIP 10.96.119.241 <none> 80/TCP,81/TCP,87/TCP,10254/TCP 2d There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is the pod port, right? Having this as port 80 might be annoying in local tests. Also, this will be exposed through a Service, which we're doing in flyteorg/flyte#3454. Reading that PR though I'm don't get why we need to use NodePort services. Isn't the external plugin service accessed only by propeller? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. with nodeport, people are able to submit the job from local. only for testing. |
||
is_flag=False, | ||
type=int, | ||
help="Grpc port for the external plugin service", | ||
) | ||
@click.option( | ||
"--worker", | ||
default="10", | ||
is_flag=False, | ||
type=int, | ||
help="Number of workers for the grpc server", | ||
) | ||
@click.option( | ||
"--timeout", | ||
default=None, | ||
is_flag=False, | ||
type=int, | ||
help="It will wait for the specified number of seconds before shutting down grpc server. It should only be used " | ||
"for testing.", | ||
) | ||
@click.pass_context | ||
def serve(_: click.Context, port, worker, timeout): | ||
""" | ||
Start a grpc server for the external plugin service. | ||
""" | ||
click.secho("Starting the external plugin service...", fg="blue") | ||
server = grpc.server(futures.ThreadPoolExecutor(max_workers=worker)) | ||
add_ExternalPluginServiceServicer_to_server(BackendPluginServer(), server) | ||
|
||
server.add_insecure_port(f"[::]:{port}") | ||
server.start() | ||
server.wait_for_termination(timeout=timeout) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,66 @@ | ||
import typing | ||
from abc import ABC, abstractmethod | ||
|
||
import grpc | ||
from flyteidl.core.tasks_pb2 import TaskTemplate | ||
from flyteidl.service.external_plugin_service_pb2 import ( | ||
RETRYABLE_FAILURE, | ||
RUNNING, | ||
SUCCEEDED, | ||
State, | ||
TaskCreateResponse, | ||
TaskDeleteResponse, | ||
TaskGetResponse, | ||
) | ||
|
||
from flytekit.models.literals import LiteralMap | ||
|
||
|
||
class BackendPluginBase(ABC): | ||
pingsutw marked this conversation as resolved.
Show resolved
Hide resolved
|
||
def __init__(self, task_type: str): | ||
self._task_type = task_type | ||
|
||
@property | ||
def task_type(self) -> str: | ||
return self._task_type | ||
|
||
@abstractmethod | ||
def create( | ||
self, | ||
context: grpc.ServicerContext, | ||
output_prefix: str, | ||
task_template: TaskTemplate, | ||
inputs: typing.Optional[LiteralMap] = None, | ||
) -> TaskCreateResponse: | ||
pass | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. like the fact that this is supposed to return a unique job id. |
||
|
||
@abstractmethod | ||
def get(self, context: grpc.ServicerContext, job_id: str) -> TaskGetResponse: | ||
pass | ||
|
||
@abstractmethod | ||
def delete(self, context: grpc.ServicerContext, job_id: str) -> TaskDeleteResponse: | ||
pass | ||
|
||
|
||
class BackendPluginRegistry(object): | ||
_REGISTRY: typing.Dict[str, BackendPluginBase] = {} | ||
|
||
@staticmethod | ||
def register(plugin: BackendPluginBase): | ||
BackendPluginRegistry._REGISTRY[plugin.task_type] = plugin | ||
pingsutw marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
@staticmethod | ||
def get_plugin(task_type: str) -> BackendPluginBase: | ||
return BackendPluginRegistry._REGISTRY[task_type] | ||
|
||
|
||
def convert_to_flyte_state(state: str) -> State: | ||
state = state.lower() | ||
if state in ["failed"]: | ||
return RETRYABLE_FAILURE | ||
elif state in ["done", "succeeded"]: | ||
return SUCCEEDED | ||
elif state in ["running"]: | ||
return RUNNING | ||
raise ValueError(f"Unrecognized state: {state}") |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
import grpc | ||
from flyteidl.service.external_plugin_service_pb2 import ( | ||
TaskCreateRequest, | ||
TaskCreateResponse, | ||
TaskDeleteRequest, | ||
TaskDeleteResponse, | ||
TaskGetRequest, | ||
TaskGetResponse, | ||
) | ||
from flyteidl.service.external_plugin_service_pb2_grpc import ExternalPluginServiceServicer | ||
|
||
from flytekit.extend.backend import model | ||
from flytekit.extend.backend.base_plugin import BackendPluginRegistry | ||
|
||
|
||
class BackendPluginServer(ExternalPluginServiceServicer): | ||
def CreateTask(self, request: TaskCreateRequest, context: grpc.ServicerContext) -> TaskCreateResponse: | ||
try: | ||
req = model.TaskCreateRequest.from_flyte_idl(request) | ||
plugin = BackendPluginRegistry.get_plugin(req.template.type) | ||
return plugin.create( | ||
context=context, inputs=req.inputs, output_prefix=req.output_prefix, task_template=req.template | ||
) | ||
except Exception as e: | ||
context.set_code(grpc.StatusCode.INTERNAL) | ||
context.set_details(f"failed to create task with error {e}") | ||
pingsutw marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
def GetTask(self, request: TaskGetRequest, context: grpc.ServicerContext) -> TaskGetResponse: | ||
try: | ||
plugin = BackendPluginRegistry.get_plugin(request.task_type) | ||
return plugin.get(context=context, job_id=request.job_id) | ||
except Exception as e: | ||
context.set_code(grpc.StatusCode.INTERNAL) | ||
context.set_details(f"failed to get task with error {e}") | ||
|
||
def DeleteTask(self, request: TaskDeleteRequest, context: grpc.ServicerContext) -> TaskDeleteResponse: | ||
try: | ||
plugin = BackendPluginRegistry.get_plugin(request.task_type) | ||
return plugin.delete(context=context, job_id=request.job_id) | ||
except Exception as e: | ||
context.set_code(grpc.StatusCode.INTERNAL) | ||
context.set_details(f"failed to delete task with error {e}") |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
from typing import Optional | ||
|
||
from flyteidl.service import external_plugin_service_pb2 | ||
|
||
from flytekit.models import common, task | ||
from flytekit.models.literals import LiteralMap | ||
|
||
|
||
class TaskCreateRequest(common.FlyteIdlEntity): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. my vote would be to get rid of this wrapper class and use the underlying IDL object directly. I started doing that once the pyi files were in. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove it |
||
def __init__(self, output_prefix: str, template: task.TaskTemplate, inputs: Optional[LiteralMap] = None): | ||
self._output_prefix = output_prefix | ||
self._template = template | ||
self._inputs = inputs | ||
|
||
@property | ||
def output_prefix(self) -> str: | ||
return self._output_prefix | ||
|
||
@property | ||
def template(self) -> task.TaskTemplate: | ||
return self._template | ||
|
||
@property | ||
def inputs(self) -> Optional[LiteralMap]: | ||
return self._inputs | ||
|
||
def to_flyte_idl(self) -> external_plugin_service_pb2.TaskCreateRequest: | ||
return external_plugin_service_pb2.TaskCreateRequest( | ||
output_prefix=self.output_prefix, | ||
template=self.template.to_flyte_idl(), | ||
inputs=self.inputs.to_flyte_idl(), | ||
) | ||
|
||
@classmethod | ||
def from_flyte_idl(cls, proto): | ||
return cls( | ||
output_prefix=proto.output_prefix, | ||
template=task.TaskTemplate.from_flyte_idl(proto.template), | ||
inputs=LiteralMap.from_flyte_idl(proto.inputs) if proto.inputs is not None else None, | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,4 +11,5 @@ | |
BigQueryTask | ||
""" | ||
|
||
from .backend_plugin import BigQueryPlugin | ||
from .task import BigQueryConfig, BigQueryTask |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,93 @@ | ||
import datetime | ||
from typing import Dict, Optional | ||
|
||
import grpc | ||
from flyteidl.service.external_plugin_service_pb2 import ( | ||
SUCCEEDED, | ||
TaskCreateResponse, | ||
TaskDeleteResponse, | ||
TaskGetResponse, | ||
) | ||
from google.cloud import bigquery | ||
|
||
from flytekit import FlyteContextManager, StructuredDataset | ||
from flytekit.core.type_engine import TypeEngine | ||
from flytekit.extend.backend.base_plugin import BackendPluginBase, BackendPluginRegistry, convert_to_flyte_state | ||
from flytekit.models import literals | ||
from flytekit.models.literals import LiteralMap | ||
from flytekit.models.task import TaskTemplate | ||
from flytekit.models.types import LiteralType, StructuredDatasetType | ||
|
||
pythonTypeToBigQueryType: Dict[type, str] = { | ||
# https://cloud.google.com/bigquery/docs/reference/standard-sql/data-types#data_type_sizes | ||
list: "ARRAY", | ||
bool: "BOOL", | ||
bytes: "BYTES", | ||
datetime.datetime: "DATETIME", | ||
float: "FLOAT64", | ||
int: "INT64", | ||
str: "STRING", | ||
} | ||
|
||
|
||
class BigQueryPlugin(BackendPluginBase): | ||
def __init__(self): | ||
super().__init__(task_type="bigquery_query_job_task") | ||
|
||
def create( | ||
self, | ||
context: grpc.ServicerContext, | ||
output_prefix: str, | ||
task_template: TaskTemplate, | ||
inputs: Optional[LiteralMap] = None, | ||
) -> TaskCreateResponse: | ||
job_config = None | ||
if inputs: | ||
ctx = FlyteContextManager.current_context() | ||
python_interface_inputs = { | ||
name: TypeEngine.guess_python_type(lt.type) for name, lt in task_template.interface.inputs.items() | ||
} | ||
native_inputs = TypeEngine.literal_map_to_kwargs(ctx, inputs, python_interface_inputs) | ||
|
||
job_config = bigquery.QueryJobConfig( | ||
query_parameters=[ | ||
pingsutw marked this conversation as resolved.
Show resolved
Hide resolved
|
||
bigquery.ScalarQueryParameter(name, pythonTypeToBigQueryType[python_interface_inputs[name]], val) | ||
eapolinario marked this conversation as resolved.
Show resolved
Hide resolved
|
||
for name, val in native_inputs.items() | ||
] | ||
) | ||
|
||
custom = task_template.custom | ||
client = bigquery.Client(project=custom["ProjectID"], location=custom["Location"]) | ||
query_job = client.query(task_template.sql.statement, job_config=job_config) | ||
|
||
return TaskCreateResponse(job_id=str(query_job.job_id)) | ||
|
||
def get(self, context: grpc.ServicerContext, job_id: str) -> TaskGetResponse: | ||
client = bigquery.Client() | ||
job = client.get_job(job_id) | ||
cur_state = convert_to_flyte_state(str(job.state)) | ||
res = None | ||
|
||
if cur_state == SUCCEEDED: | ||
ctx = FlyteContextManager.current_context() | ||
output_location = f"bq://{job.destination.project}:{job.destination.dataset_id}.{job.destination.table_id}" | ||
res = literals.LiteralMap( | ||
{ | ||
"results": TypeEngine.to_literal( | ||
ctx, | ||
StructuredDataset(uri=output_location), | ||
StructuredDataset, | ||
LiteralType(structured_dataset_type=StructuredDatasetType(format="")), | ||
) | ||
} | ||
) | ||
|
||
return TaskGetResponse(state=cur_state, outputs=res.to_flyte_idl()) | ||
|
||
def delete(self, context: grpc.ServicerContext, job_id: str) -> TaskDeleteResponse: | ||
client = bigquery.Client() | ||
client.cancel_job(job_id) | ||
return TaskDeleteResponse() | ||
|
||
|
||
BackendPluginRegistry.register(BigQueryPlugin()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's make a new package for this. don't want to confuse people with the existing flytekit base image right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
renamed it.
flytekit-external-plugin-service:latest
->external-plugin-service:latest