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.

Sign up for GitHub

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

Change Click error test assertions to be agnostic to Click version #2135

Merged
merged 2 commits into from
Mar 10, 2020
Merged

Change Click error test assertions to be agnostic to Click version #2135

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
16e7940
make click error assertions agnostic to click version
Mar 9, 2020
cc773da
fix mypy checks due to upgrade
Mar 9, 2020
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
4 changes: 2 additions & 2 deletions src/prefect/agent/agent.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ def query_flow_runs(self, tenant_id: str) -> list:
"state": {"_eq": "Running"},
"task_runs": {
"state_start_time": {
"_lte": str(now.subtract(seconds=3))
"_lte": str(now.subtract(seconds=3)) # type: ignore
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wagoodman forgot to ask: why were the typing changes necessary? Is this related to the new version of pendulum?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, there were mypy failures:

src/prefect/schedules/clocks.py:260: error: Item "None" of "Optional[Timezone]" has no attribute "name"
src/prefect/client/client.py:435: error: Incompatible types in assignment (expression has type "str", variable has type "DateTime")
src/prefect/client/client.py:482: error: Incompatible types in assignment (expression has type "str", variable has type "DateTime")
src/prefect/engine/result_handlers/s3_result_handler.py:94: error: Call to untyped function "format" in typed context
src/prefect/engine/result_handlers/gcs_result_handler.py:80: error: Call to untyped function "format" in typed context
src/prefect/engine/result_handlers/azure_result_handler.py:102: error: Call to untyped function "format" in typed context
src/prefect/agent/agent.py:384: error: Call to untyped function "subtract" in typed context
src/prefect/agent/agent.py:404: error: Call to untyped function "subtract" in typed context
src/prefect/agent/kubernetes/agent.py:24: error: Call to untyped function "timestamp" in typed context

The latest pendulum changes seem to be causing it (I think it's related to python-pendulum/pendulum#320 but didn't dig deeper):

~/code/prefect click-test-error-output*
venv381 ❯ mypy --config setup.cfg src
src/prefect/agent/kubernetes/agent.py:24: error: Call to untyped function "timestamp" in typed context
Found 1 error in 1 file (checked 179 source files)

~/code/prefect click-test-error-output* 11s
venv381 ❯ pip uninstall pendulum
Found existing installation: pendulum 2.1.0
Uninstalling pendulum-2.1.0:
...
  Successfully uninstalled pendulum-2.1.0

~/code/prefect click-test-error-output*
venv381 ❯ pip install pendulum==2.0.5
Collecting pendulum==2.0.5
...
Successfully installed pendulum-2.0.5

~/code/prefect click-test-error-output* 14s
venv381 ❯ mypy --config setup.cfg src
Success: no issues found in 179 source files

}
},
},
Expand All @@ -401,7 +401,7 @@ def query_flow_runs(self, tenant_id: str) -> list:
{
"where": {
"state_start_time": {
"_lte": str(now.subtract(seconds=3))
"_lte": str(now.subtract(seconds=3)) # type: ignore
}
}
},
Expand Down
2 changes: 1 addition & 1 deletion src/prefect/agent/kubernetes/agent.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def check_heartbeat() -> None:
"""
Check the agent's heartbeat by verifying heartbeat file has been recently modified
"""
current_timestamp = pendulum.now().timestamp()
current_timestamp = pendulum.now().timestamp() # type: ignore
last_modified_timestamp = path.getmtime("{}/heartbeat".format(AGENT_DIRECTORY))

# If file has not been modified in the last 40 seconds then raise an exit code of 1
Expand Down
4 changes: 2 additions & 2 deletions src/prefect/client/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,7 @@ def login_to_tenant(self, tenant_slug: str = None, tenant_id: str = None) -> boo
token=self._api_token,
) # type: ignore
self._access_token = payload.data.switchTenant.accessToken # type: ignore
self._access_token_expires_at = pendulum.parse(
self._access_token_expires_at = pendulum.parse( # type: ignore
payload.data.switchTenant.expiresAt # type: ignore
) # type: ignore
self._refresh_token = payload.data.switchTenant.refreshToken # type: ignore
Expand Down Expand Up @@ -479,7 +479,7 @@ def _refresh_access_token(self) -> bool:
token=self._refresh_token,
) # type: ignore
self._access_token = payload.data.refreshToken.accessToken # type: ignore
self._access_token_expires_at = pendulum.parse(
self._access_token_expires_at = pendulum.parse( # type: ignore
payload.data.refreshToken.expiresAt # type: ignore
) # type: ignore
self._refresh_token = payload.data.refreshToken.refreshToken # type: ignore
Expand Down
2 changes: 1 addition & 1 deletion src/prefect/engine/result_handlers/azure_result_handler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ def write(self, result: Any) -> str:
Returns:
- str: the Blob URI
"""
date = pendulum.now("utc").format("Y/M/D")
date = pendulum.now("utc").format("Y/M/D") # type: ignore
uri = "{date}/{uuid}.prefect_result".format(date=date, uuid=uuid.uuid4())
self.logger.debug("Starting to upload result to {}...".format(uri))

Expand Down
2 changes: 1 addition & 1 deletion src/prefect/engine/result_handlers/gcs_result_handler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ def write(self, result: Any) -> str:
Returns:
- str: the GCS URI
"""
date = pendulum.now("utc").format("Y/M/D")
date = pendulum.now("utc").format("Y/M/D") # type: ignore
uri = "{date}/{uuid}.prefect_result".format(date=date, uuid=uuid.uuid4())
self.logger.debug("Starting to upload result to {}...".format(uri))
binary_data = base64.b64encode(cloudpickle.dumps(result)).decode()
Expand Down
2 changes: 1 addition & 1 deletion src/prefect/engine/result_handlers/s3_result_handler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ def write(self, result: Any) -> str:
Returns:
- str: the S3 URI
"""
date = pendulum.now("utc").format("Y/M/D")
date = pendulum.now("utc").format("Y/M/D") # type: ignore
uri = "{date}/{uuid}.prefect_result".format(date=date, uuid=uuid.uuid4())
self.logger.debug("Starting to upload result to {}...".format(uri))

Expand Down
1 change: 1 addition & 0 deletions src/prefect/schedules/clocks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ def events(self, after: datetime = None) -> Iterable[ClockEvent]:
assert isinstance(after, datetime) # mypy assertion
after = pendulum.instance(after)
assert isinstance(after, pendulum.DateTime) # mypy assertion
assert isinstance(after.tz, pendulum.tz._Timezone) # mypy assertion

# croniter's DST logic interferes with all other datetime libraries except pytz
after_localized = pytz.timezone(after.tz.name).localize(
Expand Down
11 changes: 8 additions & 3 deletions tests/cli/test_run.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import json
import os
import re
import sys
import tempfile
from unittest.mock import MagicMock
Expand Down Expand Up @@ -220,9 +221,13 @@ def test_run_cloud_no_param_file(monkeypatch):
],
)
assert result.exit_code == 2
assert (
'Invalid value for "--parameters-file" / "-pf": Path "no_file.json" does not exist.'
in result.output
# note: click changed the output format for errors between 7.0 & 7.1, this test should be agnostic to which click version is used.
# ensure message ~= Invalid value for "--parameters-file" / "-pf": Path "no_file.json" does not exist
assert re.search(
r"Invalid value for [\"']--parameters-file", result.output, re.MULTILINE
)
assert re.search(
r"Path [\"']no_file.json[\"'] does not exist", result.output, re.MULTILINE
)


Expand Down