Match entity method parameter names #82
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.
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