[Dev 2905] Add load override for hosting capacity work packages #29
Conversation
|
Task linked: DEV-2905 Update eas-client-python |
Signed-off-by: Jimmy Tung <[email protected]>
Signed-off-by: Jimmy Tung <[email protected]>
Esaotuelr
force-pushed
the
DEV-2905-add-load-override-for-hosting-capacity-workpackage
branch
from
336fe0a to
2a395a8
Compare
roberto-marquez
left a comment
roberto-marquez
left a comment
There was a problem hiding this comment.
The part about updating the work package config itself to support the ForecastConfig and a List of FeederConfigs is completely missing. That needs to be added as it is an important part of the functionality we want.
|
|
||
| @dataclass | ||
| class FixedTimeLoadOverride: | ||
|
|
There was a problem hiding this comment.
Remove the override suffix from all these properties. It is a bit redundant given that the class itself already tells us they are an override.
| @dataclass | ||
| class TimePeriodLoadOverride: | ||
|
|
||
| load_watts_override: Optional[List[float]] |
There was a problem hiding this comment.
Same thing as the other class, remove the override suffix.
| """ | ||
| The name of the set of distribution transformer tap settings to be applied to the model from an external source. | ||
| """ | ||
|
|
There was a problem hiding this comment.
If you look at the class this is not the correct spot to put the override. Remember that the data objects for the EAS Client and the data objects we use internally are not the same. We do this to make the interface nicer for the customer.
If you look at the task the load overrides go into the FixedTime and TimePeriod classes.
Signed-off-by: Jimmy Tung <[email protected]>
Signed-off-by: Jimmy Tung <[email protected]>
Signed-off-by: Jimmy Tung <[email protected]>
|
This now has the SyfConfig change. Test was changed up to reflect. |
roberto-marquez
left a comment
roberto-marquez
left a comment
There was a problem hiding this comment.
The payloads sent by the EAS client and the expected object by the EAS server are out of sync, these are a couple of examples:
- 'syfConfig' is not defined on 'WorkPackageInput'
- 'loadTime' is not defined on 'GqlForecastConfig'.
- type is for part of 'GqlForecastConfig' or 'GqlFeederConfigs'
Essentially the integration between the client and server for the new work package doesn't work.
|
|
||
| def __init__(self, time: datetime): | ||
| self.time = time.replace(tzinfo=None) | ||
| def __init__(self, time: datetime, load_overrides: Optional[Dict[str, FixedTimeLoadOverride]]): |
There was a problem hiding this comment.
The 'load_overrides' parameter says Optional in its type hint but it doesn't have default value of None making it non optional.
| start_time: datetime, | ||
| end_time: datetime | ||
| end_time: datetime, | ||
| load_overrides: Optional[Dict[str, TimePeriodLoadOverride]] |
There was a problem hiding this comment.
The 'load_overrides' parameter says Optional in its type hint but it doesn't have default value of None making it non optional.
Signed-off-by: Jimmy Tung <[email protected]>
I've changed up the query. Functionally on the python class side it has no impact (as expected) but this should be in line with the GQL call as we moved away from union |
Signed-off-by: Jimmy Tung <[email protected]>
roberto-marquez
left a comment
roberto-marquez
left a comment
There was a problem hiding this comment.
There are naming inconsistencies that increase the likelihood is mismatches between objects in the client vs server and provide no benefit.
e.g.
loadTime vs fetchLoadTime
but we are under deadline pressure so we need to merge this PR. This should be addressed in a future PR. Maybe when we do the HCS v2 refactor.
roberto-marquez
deleted the
DEV-2905-add-load-override-for-hosting-capacity-workpackage
branch
Description
A summary of the changes and what they achieve. If there is a ClickUp task with decent detail associated with this change the summary can be brief, otherwise please add some motivation and context here.
If you have added new dependencies to the project please state why.
Associated tasks
List any other tasks / PRs that are required to be merged alongside this one (e.g. front end task for back end change or vice versa). If a PR exists, add a link. If not, just add an appropriate note here so reviewers know there is a dependency and will hold off merging until all things are ready.
Test Steps
Explain in detail how your reviewer can test the changes proposed in this PR. If it cannot be tested, leave an explanation on why.
Checklist
If any of these are not applicable, strikethrough the line
~like this~. Do not delete it!. Let the reviewer decide if you should have done it.Code
- [ ] I have added/updated unit tests for these changes, and if not I have explained why they are not necessary.Documentation
- [ ] I have updated any documentation required for these changes.Breaking Changes
Shouldn't be breaking as everything added is optional.