Adds Ruff + makes corresponding fixes #263
Conversation
| @@ -31,7 +31,6 @@ | |||
| DEFAULT_ACTION_TIMEOUT = 600 # seconds | |||
|
|
|||
|
|
|||
| DEFAULT_ACTION_LISTENER_RETRY_INTERVAL = 5 # seconds | |||
There was a problem hiding this comment.
This is imported above, so either we should remove the import or remove this line
| elif klass == object: | ||
| elif klass is object: | ||
| return self.__deserialize_object(data) | ||
| elif klass == datetime.date: | ||
| elif klass is datetime.date: | ||
| return self.__deserialize_date(data) | ||
| elif klass == datetime.datetime: | ||
| elif klass is datetime.datetime: | ||
| return self.__deserialize_datetime(data) | ||
| elif klass == decimal.Decimal: | ||
| elif klass is decimal.Decimal: |
There was a problem hiding this comment.
I'm not sure about this one - the linter flagged it, but would like eyes on this!
There was a problem hiding this comment.
update: will probably revert this just to be safe
|
|
||
| from dotenv import load_dotenv | ||
|
|
||
| from hatchet_sdk import Context, Hatchet | ||
| from hatchet_sdk.clients.admin import DedupeViolationErr | ||
| from hatchet_sdk.loader import ClientConfig | ||
|
|
||
| load_dotenv() |
There was a problem hiding this comment.
note for future: i personally dont like this here... i like how npm runs this in the process before running the example
There was a problem hiding this comment.
yeah, you mean the load_dotenv? I think we'd benefit from using Pydantic Settings (Pydantic everywhere!) in the examples: https://docs.pydantic.dev/latest/concepts/pydantic_settings/#dotenv-env-support
| @@ -31,7 +31,6 @@ | |||
| DEFAULT_ACTION_TIMEOUT = 600 # seconds | |||
|
|
|||
|
|
|||
| DEFAULT_ACTION_LISTENER_RETRY_INTERVAL = 5 # seconds | |||
There was a problem hiding this comment.
im not sure myself, but should we expose these if the user hits this case?
| @@ -1 +1 @@ | |||
| from .context import Context | |||
| from .context import Context as Context | |||
| @@ -188,7 +188,6 @@ def cleanup_subscription(self, subscription_id: int): | |||
| del self.events[subscription_id] | |||
|
|
|||
| async def subscribe(self, workflow_run_id: str): | |||
| init_producer: asyncio.Task = None | |||
There was a problem hiding this comment.
assuming this removed because its unused, but double check
| @@ -11,10 +11,6 @@ | |||
| from hatchet_sdk.clients.workflow_listener import PooledWorkflowRunListener | |||
| from hatchet_sdk.context.worker_context import WorkerContext | |||
| from hatchet_sdk.contracts.dispatcher_pb2 import OverridesData | |||
| from hatchet_sdk.contracts.workflows_pb2 import ( | |||
There was a problem hiding this comment.
i think where exposing these here for folks to import in a convenient way ... should be type hinted below
| @@ -39,7 +39,7 @@ def __post_init__(self): | |||
| self.start() | |||
|
|
|||
| def start(self, retry_count=1): | |||
| k = self.loop.create_task(self.async_start(retry_count)) | |||
| self.loop.create_task(self.async_start(retry_count)) | |||
There was a problem hiding this comment.
i think we had some issues with "no ref to future" without the k and this at least silenced it... not sure what the right way of doing this is/if its not an issue
Adding
ruff(super fast Python linter) and making corresponding fixes. Most of the fixes are just removing unused imports and variables, getting rid of assignments that don't get used, etc.