prefect icon indicating copy to clipboard operation
prefect copied to clipboard

Prefect package dependency on Griffe is spilling into user software

Open mahiki opened this issue 1 year ago • 2 comments

Bug summary

I am deploying a flow from a prefect.yaml file with the following:

prefect --no-prompt deploy --all --prefect-file ./build/projects/decks/prefect.yaml

╭──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮
│ Deploying wbr_gne                                                                                                                    │
╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯
Defining a schedule via the `schedule` key in the deployment has been deprecated. It will not be available after Sep 2024. Please use
`schedules` instead by renaming the `schedule` key to `schedules` and providing a list of schedule objects.
/Users/merlinr/Library/Caches/pypoetry/virtualenvs/templisher-4hu7XBP5-py3.11/lib/python3.11/site-packages/prefect/utilities/importtools.py:511: RuntimeWarning: coroutine 'Block.load' was never awaited
  logger.debug("Failed to compile: %s", e)
RuntimeWarning: Enable tracemalloc to get the object allocation traceback
19:26:50.774 | WARNING | griffe - <module>:3: No type or annotation for parameter 'deck_id'
19:26:50.775 | WARNING | griffe - <module>:4: Failed to get 'name: description' pair from 'deck_id.yaml.'
19:26:50.775 | WARNING | griffe - <module>:5: No type or annotation for parameter 'stage_exclude_list'
/Users/merlinr/Library/Caches/pypoetry/virtualenvs/templisher-4hu7XBP5-py3.11/lib/python3.11/site-packages/prefect/flows.py:1693: RuntimeWarning: coroutine 'Block.load' was never awaited
  if flow is None:
RuntimeWarning: Enable tracemalloc to get the object allocation traceback
19:26:50.776 | WARNING | griffe - <module>:3: No type or annotation for parameter 'deck_id'
19:26:50.776 | WARNING | griffe - <module>:4: Failed to get 'name: description' pair from 'deck_id.yaml.'
19:26:50.776 | WARNING | griffe - <module>:5: No type or annotation for parameter 'stage_exclude_list'
1 validation error for MinimalDeploymentSchedule
active
  value could not be parsed to a boolean (type=type_error.bool)

We can ignore the warning about coroutine not awaited.

Why am I getting warnings about what's in the docstrings of the data application I'm building? What's next, enforcing Black formatting?

Absolutely fine to use Pydantic, Griffe, Black, whatever you want in the Prefect package, fantastic. But there needs to be some border between my application code and the orchestrator which is a dependency.

Version info (prefect version output)

poetry run prefect version
Version:             2.20.3
API version:         0.8.4
Python version:      3.11.9
Git commit:          b8c27aa0
Built:               Thu, Aug 22, 2024 3:13 PM
OS/Arch:             darwin/arm64
Profile:             dev
Server type:         server

Additional context

I love Prefect, been investing a lot and I can't wait for 3.0.

mahiki avatar Aug 27 '24 02:08 mahiki

something strange going on with those two obvious bots responding above.

mahiki avatar Aug 27 '24 02:08 mahiki

(wave of similar comments on GitHub, report if you can, otherwise ignore)

Regarding the Griffe warnings you're getting, I believe Prefect previously filtered those out, but latest Griffe changed its logger name, so maybe the filter isn't working anymore. Can be fixed easily by using the "griffe" logger name when filtering :) Or even easier, by using with griffe.logger.disable(): ....

pawamoy avatar Aug 27 '24 11:08 pawamoy

Yea, I agree these logs should be filtered out; for the sake of understanding the use of griffe here, this isn't being used for linting. It's used to parse the block docstring for formatted display in the UI - Prefect separates out the core docstring from any examples of usage. Having a whole dependency just for doing this might be overkill but we probably won't have time to revisit that for a little while.

I do think that documentation of the orchestration objects you create (blocks, deployments, tasks, etc.) are important to the data application you build, especially if it has downstream users.

cicdw avatar Aug 28 '24 17:08 cicdw

@cicdw there's a lighter dependency, docstring_parser, but I don't think it's able to pick up type annotations from the signature.

pawamoy avatar Aug 28 '24 17:08 pawamoy