Conversation
- Ensure consistency in worker.py to task.py
There was a problem hiding this comment.
Pull request overview
This PR addresses parameter naming inconsistencies between the abstract base class methods in task.py and their implementations in worker.py for entity-related operations. The changes ensure that overridden methods use the same parameter names as their base class counterparts, improving code consistency and compliance with proper inheritance practices.
Key Changes:
- Updated
call_entitymethod parameter fromentity_idtoentityto match the abstract method signature - Updated
signal_entitymethod parameter fromoperationtooperation_nameto match the abstract method signature
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| durabletask/worker.py | Updated parameter names in call_entity (entity_id → entity) and signal_entity (operation → operation_name) to match base class signatures |
| durabletask/task.py | Minor formatting change to call_entity abstract method signature (line break adjustment) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self.call_entity_function_helper( | ||
| id, entity_id, operation, input=input | ||
| id, entity, operation, input=input |
There was a problem hiding this comment.
For consistency and maintainability, the helper function call_entity_function_helper should have its parameter name updated from entity_id (line 1155) to entity to match the public method parameter name. While the code works because arguments are passed positionally, this naming inconsistency can cause confusion during maintenance.
| self.signal_entity_function_helper( | ||
| id, entity_id, operation, input | ||
| id, entity_id, operation_name, input |
There was a problem hiding this comment.
For consistency and maintainability, the helper function signal_entity_function_helper should have its parameter name updated from operation (line 1178) to operation_name to match the public method parameter name. While the code works because arguments are passed positionally, this naming inconsistency can cause confusion during maintenance.
2 participants
Ensure consistency in worker.py to task.py - currently the overrides in worker.py are incompatible with the base methods in task.py due to the parameter names being different.
This one irks me because when I implemented this, I was inconsistent between call_entity and signal_entity, but any change in task.py is a breaking change