explicit exports for symbols used in flows

[tekumara]

Description

from prefect import Flow, task
from prefect.executors import DaskExecutor
from prefect.run_configs import KubernetesRun
from prefect.storage import S3

Generates the following pyright errors:

  flow.py:1:21 - error: "Flow" is not exported from module "prefect"
    Import from "prefect.core" instead (reportPrivateImportUsage)
  flow.py:1:27 - error: "task" is not exported from module "prefect"
    Import from "prefect.utilities.tasks" instead (reportPrivateImportUsage)
  flow.py:2:31 - error: "DaskExecutor" is not exported from module "prefect.executors"
    Import from "prefect.executors.dask" instead (reportPrivateImportUsage)
  flow.py:3:33 - error: "KubernetesRun" is not exported from module "prefect.run_configs"
    Import from "prefect.run_configs.kubernetes" instead (reportPrivateImportUsage)
  flow.py:4:29 - error: "S3" is not exported from module "prefect.storage"

pyright (and therefore vscode) considers these symbols private based on these rules.

PEP 008 states modules should explicitly declare the names in their public API using the __all__ attribute .
PEP 484 also mentions that imported modules are not considered exported unless they use the import ... as ... syntax or equivalent.

Environment

prefect version 0.15.9

[tekumara]

The impact of this issue is that we cannot use pyright on our flow codebases.

[tekumara]

Or we downgrade to before pyright 1.1.168

[zanieb]

We do not support pyright and I'm hesitant to make changes to accommodate it. If you want to do a PoC of resolving these errors with minimal changes, I'm willing to consider it. I think the easiest solution is to just use the pyright configuration to change these reports from 'error' to another level.

https://github.com/microsoft/pyright/blob/main/docs/configuration.md

[tekumara]

Ah yes, we could disable reportPrivateImportUsage in the meantime.

FYI: mypy also errors for the same reason when using --strict:

$ mypy --strict flow.py
flow.py:1: error: Module "prefect" does not explicitly export attribute "Flow"; implicit reexport disabled
flow.py:1: error: Module "prefect" does not explicitly export attribute "task"; implicit reexport disabled
flow.py:2: error: Module "prefect.executors" does not explicitly export attribute "DaskExecutor"; implicit reexport disabled
flow.py:3: error: Module "prefect.run_configs" does not explicitly export attribute "KubernetesRun"; implicit reexport disabled
flow.py:4: error: Module "prefect.storage" does not explicitly export attribute "S3"; implicit reexport disabled

[aripollak]

@madkinsz according to https://github.com/microsoft/pyright/blob/main/docs/typed-libraries.md#library-interface, it wouldn't consider these modules as private if they're imported like from .core import Task instead of from prefect.core import Task. If I made a PR to change these, would that be an acceptable fix?

[zanieb]

We use absolute path imports like that everywhere so I'd prefer not. You could add an __all__ definition to the main __init__ file though.

[aripollak]

Hm, are you sure you'd rather use __all__? It seems like that would more easily become out-of-date.

[tekumara]

The other way to signal a symbol is exported is with a redundant module alias, eg:

from prefect.core import Task as Task

[zanieb]

I believe __all__ is the most inline with Python convention here. If you're going to contribute a fix, I'd appreciate if it was to 1.0.0-rc1 branch as that should be the stable API going forward and will not change for quite some time.

[tekumara]

Fixed in #5293