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

IWF-157: Fix typo IdReusePolicy of ALLOW_IF_PREVIOUS_EXISTS_ABNORMALLY #33

Merged
merged 15 commits into from
Oct 1, 2024

Conversation

stevo89519
Copy link
Contributor

No description provided.

@stevo89519 stevo89519 marked this pull request as draft September 26, 2024 19:08
@stevo89519 stevo89519 marked this pull request as ready for review September 26, 2024 19:11
@longquanzheng

This comment was marked as resolved.

@stevo89519

This comment was marked as resolved.

@longquanzheng
Copy link
Contributor

Can you look into why the tests failing

@longquanzheng Fixed some linter warnings. Tests passed by themselves (no changes). Linter is having dependency issues: Error: Command failed: poetry install

I guess we need to upgrade this to use official action: https://github.com/marketplace/actions/install-poetry-action

This is no longer working properly with old nodeJs: knowsuchagency/poetry-install#4

@longquanzheng

This comment was marked as resolved.

env:
POETRY_VIRTUALENVS_CREATE: false
with:
virtualenvs-create: false
Copy link
Contributor

Choose a reason for hiding this comment

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

I copied this value from somewhere else but didn't fully understand. do you know why this has to be 'false'?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No I don't know. I'm also not sure if calling 'poetry install' here will cause any downsides? It fixed the workflow issues.

@@ -291,7 +291,7 @@ def default(self, o: Any) -> Any:
See :py:meth:`json.JSONEncoder.default`.
"""
# Dataclass support
if dataclasses.is_dataclass(o):
if dataclasses.is_dataclass(o) and not isinstance(o, type):
Copy link
Contributor Author

@stevo89519 stevo89519 Sep 30, 2024

Choose a reason for hiding this comment

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

Fixes mypy error:
No overload variant of "asdict" matches argument type "Type[DataclassInstance]" [call-overload]

Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't see this error before.
Did you find this fix somewhere? can you put a reference here for more context?

Copy link
Contributor Author

@stevo89519 stevo89519 Oct 1, 2024

Choose a reason for hiding this comment

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

Found the fix here: python/mypy#17550 (comment)

Copy link
Contributor

@longquanzheng longquanzheng left a comment

Choose a reason for hiding this comment

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

Great job!

@stevo89519 stevo89519 merged commit de39f87 into main Oct 1, 2024
2 checks passed
@stevo89519 stevo89519 deleted the jira/sharrison/IWF-157 branch October 3, 2024 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants