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.

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

Moves experimental_warn setting to experiments model and removes async_fetch_state_result #4

Open
wants to merge 1 commit into
base: frances/test_promptless3
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 12 additions & 32 deletions docs/3.0/develop/settings-ref.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -164,38 +164,6 @@ The URL of the Prefect UI. If not set, the client will attempt to infer it.
**Supported environment variables**:
`PREFECT_SILENCE_API_URL_MISCONFIGURATION`

### `experimental_warn`
If `True`, warn on usage of experimental features.

**Type**: `boolean`

**Default**: `True`

**TOML dotted key path**: `experimental_warn`

**Supported environment variables**:
`PREFECT_EXPERIMENTAL_WARN`

### `async_fetch_state_result`

Determines whether `State.result()` fetches results automatically or not.
In Prefect 2.6.0, the `State.result()` method was updated to be async
to facilitate automatic retrieval of results from storage which means when
writing async code you must `await` the call. For backwards compatibility,
the result is not retrieved by default for async users. You may opt into this
per call by passing `fetch=True` or toggle this setting to change the behavior
globally.


**Type**: `boolean`

**Default**: `False`

**TOML dotted key path**: `async_fetch_state_result`

**Supported environment variables**:
`PREFECT_ASYNC_FETCH_STATE_RESULT`

---
## APISettings
Settings for interacting with the Prefect API
Expand Down Expand Up @@ -475,6 +443,18 @@ The default Docker namespace to use when building images.
---
## ExperimentsSettings
Settings for configuring experimental features
### `warn`
If `True`, warn on usage of experimental features.

**Type**: `boolean`

**Default**: `True`

**TOML dotted key path**: `experiments.warn`

**Supported environment variables**:
`PREFECT_EXPERIMENTS_WARN`, `PREFECT_EXPERIMENTAL_WARN`

### `worker_logging_to_api_enabled`
Enables the logging of worker logs to Prefect Cloud.

Expand Down
28 changes: 10 additions & 18 deletions schemas/settings.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,16 @@
"ExperimentsSettings": {
"description": "Settings for configuring experimental features",
"properties": {
"warn": {
"default": true,
"description": "If `True`, warn on usage of experimental features.",
"supported_environment_variables": [
"PREFECT_EXPERIMENTS_WARN",
"PREFECT_EXPERIMENTAL_WARN"
],
"title": "Warn",
"type": "boolean"
},
"worker_logging_to_api_enabled": {
"default": false,
"description": "Enables the logging of worker logs to Prefect Cloud.",
Expand Down Expand Up @@ -2217,24 +2227,6 @@
],
"title": "Silence Api Url Misconfiguration",
"type": "boolean"
},
"experimental_warn": {
"default": true,
"description": "If `True`, warn on usage of experimental features.",
"supported_environment_variables": [
"PREFECT_EXPERIMENTAL_WARN"
],
"title": "Experimental Warn",
"type": "boolean"
},
"async_fetch_state_result": {
"default": false,
"description": "\n Determines whether `State.result()` fetches results automatically or not.\n In Prefect 2.6.0, the `State.result()` method was updated to be async\n to facilitate automatic retrieval of results from storage which means when\n writing async code you must `await` the call. For backwards compatibility,\n the result is not retrieved by default for async users. You may opt into this\n per call by passing `fetch=True` or toggle this setting to change the behavior\n globally.\n ",
"supported_environment_variables": [
"PREFECT_ASYNC_FETCH_STATE_RESULT"
],
"title": "Async Fetch State Result",
"type": "boolean"
}
},
"title": "Prefect Settings",
Expand Down
7 changes: 0 additions & 7 deletions src/integrations/prefect-dask/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

import pytest

from prefect.settings import PREFECT_ASYNC_FETCH_STATE_RESULT, temporary_settings
from prefect.testing.utilities import prefect_test_harness


Expand Down Expand Up @@ -46,9 +45,3 @@ def event_loop(request):
# Workaround for failures in pytest_asyncio 0.17;
# see https://github.com/pytest-dev/pytest-asyncio/issues/257
policy.set_event_loop(loop)


@pytest.fixture(autouse=True)
def fetch_state_result():
with temporary_settings(updates={PREFECT_ASYNC_FETCH_STATE_RESULT: True}):
yield
7 changes: 0 additions & 7 deletions src/integrations/prefect-ray/tests/conftest.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,9 @@
import pytest

from prefect.settings import PREFECT_ASYNC_FETCH_STATE_RESULT, temporary_settings
from prefect.testing.utilities import prefect_test_harness


@pytest.fixture(scope="session", autouse=True)
def prefect_db():
with prefect_test_harness():
yield


@pytest.fixture(autouse=True)
def fetch_state_result():
with temporary_settings(updates={PREFECT_ASYNC_FETCH_STATE_RESULT: True}):
yield
7 changes: 0 additions & 7 deletions src/integrations/prefect-sqlalchemy/tests/conftest.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import pytest

from prefect.settings import PREFECT_ASYNC_FETCH_STATE_RESULT, temporary_settings
from prefect.testing.utilities import prefect_test_harness


Expand All @@ -11,9 +10,3 @@ def prefect_db():
"""
with prefect_test_harness():
yield


@pytest.fixture(autouse=True)
def fetch_state_result():
with temporary_settings(updates={PREFECT_ASYNC_FETCH_STATE_RESULT: True}):
yield
28 changes: 10 additions & 18 deletions src/prefect/client/schemas/objects.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
from pydantic_extra_types.pendulum_dt import DateTime
from typing_extensions import Literal, Self, TypeVar

from prefect._internal.compatibility import deprecated
from prefect._internal.compatibility.migration import getattr_migration
from prefect._internal.schemas.bases import ObjectBaseModel, PrefectBaseModel
from prefect._internal.schemas.fields import CreatedBy, UpdatedBy
Expand Down Expand Up @@ -220,10 +221,17 @@ def result(self: "State[R]", raise_on_failure: bool = True) -> R:
def result(self: "State[R]", raise_on_failure: bool = False) -> Union[R, Exception]:
...

@deprecated.deprecated_parameter(
"fetch",
when=lambda fetch: fetch is not True,
start_date="Oct 2024",
end_date="Jan 2025",
help="Please ensure you are awaiting the call to `result()` when calling in an async context.",
)
def result(
self,
raise_on_failure: bool = True,
fetch: Optional[bool] = None,
fetch: bool = True,
retry_result_failure: bool = True,
) -> Union[R, Exception]:
"""
Expand All @@ -248,22 +256,6 @@ def result(
The result of the run

Examples:
>>> from prefect import flow, task
>>> @task
>>> def my_task(x):
>>> return x

Get the result from a task future in a flow

>>> @flow
>>> def my_flow():
>>> future = my_task("hello")
>>> state = future.wait()
>>> result = state.result()
>>> print(result)
>>> my_flow()
hello

Get the result from a flow state

>>> @flow
Expand Down Expand Up @@ -307,7 +299,7 @@ def result(
>>> raise ValueError("oh no!")
>>> my_flow.deploy("my_deployment/my_flow")
>>> flow_run = run_deployment("my_deployment/my_flow")
>>> await flow_run.state.result(raise_on_failure=True, fetch=True) # Raises `ValueError("oh no!")`
>>> await flow_run.state.result(raise_on_failure=True) # Raises `ValueError("oh no!")`
"""
from prefect.states import get_state_result

Expand Down
10 changes: 9 additions & 1 deletion src/prefect/settings/models/experiments.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from pydantic import Field
from pydantic import AliasChoices, AliasPath, Field

from prefect.settings.base import PrefectBaseSettings, _build_settings_config

Expand All @@ -10,6 +10,14 @@ class ExperimentsSettings(PrefectBaseSettings):

model_config = _build_settings_config(("experiments",))

warn: bool = Field(
default=True,
description="If `True`, warn on usage of experimental features.",
validation_alias=AliasChoices(
AliasPath("warn"), "prefect_experiments_warn", "prefect_experimental_warn"
),
)

worker_logging_to_api_enabled: bool = Field(
default=False,
description="Enables the logging of worker logs to Prefect Cloud.",
Expand Down
19 changes: 0 additions & 19 deletions src/prefect/settings/models/root.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,25 +149,6 @@ class Settings(PrefectBaseSettings):
""",
)

experimental_warn: bool = Field(
default=True,
description="If `True`, warn on usage of experimental features.",
)

# this setting needs to be removed
async_fetch_state_result: bool = Field(
default=False,
description="""
Determines whether `State.result()` fetches results automatically or not.
In Prefect 2.6.0, the `State.result()` method was updated to be async
to facilitate automatic retrieval of results from storage which means when
writing async code you must `await` the call. For backwards compatibility,
the result is not retrieved by default for async users. You may opt into this
per call by passing `fetch=True` or toggle this setting to change the behavior
globally.
""",
)

###########################################################################
# allow deprecated access to PREFECT_SOME_SETTING_NAME

Expand Down
39 changes: 20 additions & 19 deletions src/prefect/states.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import pendulum
from typing_extensions import TypeGuard

from prefect._internal.compatibility import deprecated
from prefect.client.schemas import State as State
from prefect.client.schemas import StateDetails, StateType
from prefect.exceptions import (
Expand All @@ -32,18 +33,24 @@
ResultRecordMetadata,
ResultStore,
)
from prefect.settings import PREFECT_ASYNC_FETCH_STATE_RESULT
from prefect.utilities.annotations import BaseAnnotation
from prefect.utilities.asyncutils import in_async_main_thread, sync_compatible
from prefect.utilities.collections import ensure_iterable

logger = get_logger("states")


@deprecated.deprecated_parameter(
"fetch",
when=lambda fetch: fetch is not True,
start_date="Oct 2024",
end_date="Jan 2025",
help="Please ensure you are awaiting the call to `result()` when calling in an async context.",
)
def get_state_result(
state: State[R],
raise_on_failure: bool = True,
fetch: Optional[bool] = None,
fetch: bool = True,
retry_result_failure: bool = True,
) -> R:
"""
Expand All @@ -52,23 +59,17 @@ def get_state_result(
See `State.result()`
"""

if fetch is None and (
PREFECT_ASYNC_FETCH_STATE_RESULT or not in_async_main_thread()
):
# Fetch defaults to `True` for sync users or async users who have opted in
fetch = True
if not fetch:
if fetch is None and in_async_main_thread():
warnings.warn(
(
"State.result() was called from an async context but not awaited. "
"This method will be updated to return a coroutine by default in "
"the future. Pass `fetch=True` and `await` the call to get rid of "
"this warning."
),
DeprecationWarning,
stacklevel=2,
)
if not fetch and in_async_main_thread():
warnings.warn(
(
"State.result() was called from an async context but not awaited. "
"This method will be updated to return a coroutine by default in "
"the future. Pass `fetch=True` and `await` the call to get rid of "
"this warning."
),
DeprecationWarning,
stacklevel=2,
)

return state.data
else:
Expand Down
2 changes: 0 additions & 2 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@
PREFECT_API_SERVICES_TASK_RUN_RECORDER_ENABLED,
PREFECT_API_SERVICES_TRIGGERS_ENABLED,
PREFECT_API_URL,
PREFECT_ASYNC_FETCH_STATE_RESULT,
PREFECT_CLI_COLORS,
PREFECT_CLI_WRAP_LINES,
PREFECT_HOME,
Expand Down Expand Up @@ -323,7 +322,6 @@ def pytest_sessionstart(session):
PREFECT_CLI_COLORS: False,
PREFECT_CLI_WRAP_LINES: False,
# Enable future change
PREFECT_ASYNC_FETCH_STATE_RESULT: True,
# Enable debug logging
PREFECT_LOGGING_LEVEL: "DEBUG",
PREFECT_LOGGING_INTERNAL_LEVEL: "DEBUG",
Expand Down
18 changes: 1 addition & 17 deletions tests/results/test_result_fetch.py
Original file line number Diff line number Diff line change
@@ -1,21 +1,6 @@
import pytest

from prefect import flow, task
from prefect.settings import PREFECT_ASYNC_FETCH_STATE_RESULT, temporary_settings


@pytest.fixture(autouse=True)
def disable_fetch_by_default():
"""
The test suite defaults to the future behavior.

For these tests, we enable the default user behavior.
"""
with temporary_settings({PREFECT_ASYNC_FETCH_STATE_RESULT: False}):
yield


@pytest.mark.skip(reason="This test is flaky and needs to be fixed")
async def test_async_result_warnings_are_not_raised_by_engine():
# Since most of our tests are run with the opt-in globally enabled, this test
# covers a bunch of features to cover remaining cases where we may internally
Expand Down Expand Up @@ -92,8 +77,7 @@ async def foo():
return 1

state = await foo(return_state=True)
with temporary_settings({PREFECT_ASYNC_FETCH_STATE_RESULT: True}):
coro = state.result(fetch=True)
coro = state.result()

assert await coro == 1

Expand Down
4 changes: 2 additions & 2 deletions tests/test_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,6 @@
"PREFECT_API_TASK_CACHE_KEY_MAX_LENGTH": {"test_value": 10, "legacy": True},
"PREFECT_API_TLS_INSECURE_SKIP_VERIFY": {"test_value": True},
"PREFECT_API_URL": {"test_value": "https://api.prefect.io"},
"PREFECT_ASYNC_FETCH_STATE_RESULT": {"test_value": True},
"PREFECT_CLIENT_CSRF_SUPPORT_ENABLED": {"test_value": True},
"PREFECT_CLIENT_ENABLE_METRICS": {"test_value": True, "legacy": True},
"PREFECT_CLIENT_MAX_RETRIES": {"test_value": 3},
Expand Down Expand Up @@ -227,7 +226,8 @@
"legacy": True,
},
"PREFECT_EVENTS_WEBSOCKET_BACKFILL_PAGE_SIZE": {"test_value": 10, "legacy": True},
"PREFECT_EXPERIMENTAL_WARN": {"test_value": True},
"PREFECT_EXPERIMENTAL_WARN": {"test_value": True, "legacy": True},
"PREFECT_EXPERIMENTS_WARN": {"test_value": True},
"PREFECT_EXPERIMENTS_WORKER_LOGGING_TO_API_ENABLED": {"test_value": False},
"PREFECT_FLOW_DEFAULT_RETRIES": {"test_value": 10, "legacy": True},
"PREFECT_FLOWS_DEFAULT_RETRIES": {"test_value": 10},
Expand Down
Loading