prefect icon indicating copy to clipboard operation
prefect copied to clipboard

Proposal to move prefect REST API client to separate package/library

Open xnuinside opened this issue 3 years ago • 12 comments

First check

  • [X] I added a descriptive title to this issue.
  • [X] I used the GitHub search to find a similar request and didn't find it.
  • [X] I searched the Prefect documentation for this feature.

Prefect Version

2.x

Describe the proposed behavior

First of all, thanks for the great tool.

This issue is a request to have separate prefect-client library to interact with Prefect REST API. Or split prefect.client from current library.

You call REST API of some server usually from the client, not server itself, and there is no any need to get such huge package as prefect with all server-logic required staff

in some cases it is critical to think about package size and dependencies, for example, like in AWS Lambdas

in will be great to have a light-weight python-client for Prefect REST API without need load all server-dependencies

Because, other way, we need to write our own 'providers'/'adapters' to interact with API to avoid load huge package in dependencies, like here https://github.com/geobeyond/fastflows/blob/develop/fastflows/providers/prefect.py#L50.

If maintainers will be ok with that - we can help with moving REST API client to separate prefect-client library. Ideally it should have in deps only httpx, pydantic, and/or minor deps only for developing proper REST API python client.

Describe the current behavior

install whole prefect package

Example Use

I has AWS Lambda that need to get state of prefect flow.

Or I have a backend server that use prefect API for some feature, or I create a package that interact with different flows managers as an upper layer. For example, https://github.com/geobeyond/fastflows/blob/develop/fastflows/providers/prefect.py#L50

Additional context

No response

xnuinside avatar Aug 08 '22 16:08 xnuinside

Hi! Thanks for this well written issue. I think your suggestion has a lot of merit, but a change like this is something that we'll need to discuss as a team.

A couple concerns off the top of my head:

  • The client supports connection directly to an in-memory FastAPI application, which means we'd still have that dependency or we'd have to introduce a separate client implementation.
  • The client makes use of many utilities from the core library, some of which we probably do not want to replicate but it'd be weird for the core library to import them from the client implementation.

zanieb avatar Aug 08 '22 17:08 zanieb

@madkinsz I also thought about it when I look at source code, so second thought was not to touch current prefect package, but create fresh new prefect-client package based on open api configuration that already exists in prefect (possible to try something with autogeneration from OpenAPI). But anyway will be great to hear team feedback. And thanks for the quick response!

xnuinside avatar Aug 08 '22 17:08 xnuinside

I'm going to mark this as deferred for now, as I do not think we can undertake this in the near future.

Loosely, I think that creating a prefect-openapi-client is something we're interested in. I imagine a package like

prefect_openapi_client
    python
       prefect_openapi_client
          ...
       setup.py

with auto-generated clients for different languages from our OpenAPI specifications would be great.

zanieb avatar Aug 09 '22 16:08 zanieb

@madkinsz can I ask why still use setup.py + setup.cfg and not modern pyproject.toml & for example, poetry?

xnuinside avatar Aug 11 '22 11:08 xnuinside

there is a bunch of issues in Prefect open api config Screenshot 2022-08-11 at 14 19 16 need to fix them firstly to make autogeneration works )

for example:

Screenshot 2022-08-11 at 14 16 11

in Open Api 3.0 'null' is not a type but it is defined as a response type in schema in endpoints /deployments/{id}/set_schedule_inactive & /deployments/{id}/set_schedule_active

xnuinside avatar Aug 11 '22 11:08 xnuinside

We don't have internal consensus on alternative package manager to use an pip / setuptools has all the features we need.

What are you using to detect the OpenAPI errors?

zanieb avatar Aug 11 '22 13:08 zanieb

setuptools also support pyproject toml https://setuptools.pypa.io/en/latest/userguide/pyproject_config.html ) setup.cfg & setup.py is pretty old & not actual since PEP, pip also support pyproject.toml https://pip.pypa.io/en/stable/reference/build-system/pyproject-toml/

but anyway

about errors - I tried generator - https://github.com/openapi-generators/openapi-python-client

Full list OpenApi issues:


Failed to parse OpenAPI document

9 validation errors for OpenAPI
paths -> /deployments/{id}/schedule -> post -> responses -> 200 -> content -> application/json -> schema -> $ref
  field required (type=value_error.missing)
paths -> /deployments/{id}/schedule -> post -> responses -> 200 -> content -> application/json -> schema -> type
  value is not a valid enumeration member; permitted: 'string', 'number', 'integer', 'boolean', 'array', 'object' (type=type_error.enum; enum_values=[<DataType.STRING: 'string'>, <DataType.NUMBER: 'number'>, <DataType.INTEGER: 'integer'>, <DataType.BOOLEAN: 'boolean'>, <DataType.ARRAY: 'array'>, <DataType.OBJECT: 'object'>])
paths -> /deployments/{id}/schedule -> post -> responses -> 200 -> $ref
  field required (type=value_error.missing)
paths -> /deployments/{id}/set_schedule_active -> post -> responses -> 200 -> content -> application/json -> schema -> $ref
  field required (type=value_error.missing)
paths -> /deployments/{id}/set_schedule_active -> post -> responses -> 200 -> content -> application/json -> schema -> type
  value is not a valid enumeration member; permitted: 'string', 'number', 'integer', 'boolean', 'array', 'object' (type=type_error.enum; enum_values=[<DataType.STRING: 'string'>, <DataType.NUMBER: 'number'>, <DataType.INTEGER: 'integer'>, <DataType.BOOLEAN: 'boolean'>, <DataType.ARRAY: 'array'>, <DataType.OBJECT: 'object'>])
paths -> /deployments/{id}/set_schedule_active -> post -> responses -> 200 -> $ref
  field required (type=value_error.missing)
paths -> /deployments/{id}/set_schedule_inactive -> post -> responses -> 200 -> content -> application/json -> schema -> $ref
  field required (type=value_error.missing)
paths -> /deployments/{id}/set_schedule_inactive -> post -> responses -> 200 -> content -> application/json -> schema -> type
  value is not a valid enumeration member; permitted: 'string', 'number', 'integer', 'boolean', 'array', 'object' (type=type_error.enum; enum_values=[<DataType.STRING: 'string'>, <DataType.NUMBER: 'number'>, <DataType.INTEGER: 'integer'>, <DataType.BOOLEAN: 'boolean'>, <DataType.ARRAY: 'array'>, <DataType.OBJECT: 'object'>])
paths -> /deployments/{id}/set_schedule_inactive -> post -> responses -> 200 -> $ref
  field required (type=value_error.missing)

all of them correct in scope current OpenAPI 3.x standard

xnuinside avatar Aug 11 '22 14:08 xnuinside

@madkinsz also about setuptools & setup.cfg - https://github.com/pypa/setuptools/issues/3214 they plan to deprecate it & milestone for it https://github.com/pypa/setuptools/milestone/7

but setup.py is used for sure ) question only in setup.cfg

xnuinside avatar Aug 11 '22 14:08 xnuinside

about errors - I tried generator - https://github.com/openapi-generators/openapi-python-client

Thanks! @abrookins should we open an issue to ensure our OpenAPI specification conforms to the latest standards and include some sort of test?

also about setuptools & setup.cfg - https://github.com/pypa/setuptools/issues/3214 they plan to deprecate it & milestone for it https://github.com/pypa/setuptools/milestone/7

Yeah but they haven't deprecated it yet and they noting that they want to wait for pyproject.toml to stabilize first. This isn't really affecting our developer experience and we'd rather have a stable and familiar config for now. We are likely to move to the new formats sometime in the future.

zanieb avatar Aug 12 '22 15:08 zanieb

@madkinsz Yes, let's open an issue to sort out what's happening here. We should be able to create a unit test that validates the output of our OpenAPI spec against the supported version (3.0.2 AFAIK, from FastAPI).

abrookins avatar Aug 12 '22 15:08 abrookins

I created https://github.com/PrefectHQ/prefect/issues/6400 to track validating our OpenAPI spec.

abrookins avatar Aug 12 '22 15:08 abrookins

This issue is stale because it has been open 30 days with no activity. To keep this issue open remove stale label or comment.

github-actions[bot] avatar Sep 14 '22 02:09 github-actions[bot]

This issue was closed because it has been stale for 7 days with no activity. If this issue is important or you have more to add feel free to re-open it.

github-actions[bot] avatar Sep 28 '22 02:09 github-actions[bot]