Feature/research 306#1918
Conversation
… external strategy to support multiple orders.
…WDS external strategy, to communicate exclusively with Energy Web Digital Spine.
…Web Client Gateway.
…d EWCGStrategy class.
… the Intelligent Digital Spine.
… external strategies.
hannesdiedrich
left a comment
hannesdiedrich
left a comment
There was a problem hiding this comment.
I have left some comments and questions. I have a general one before I can continue
I started with the EWCG strategy only to realise that this is not the external strategy (my bad).
I understand the gsy "hosts" the rest API for any external commands that are performed via the rest interface. And the Energy Web Client Gateway acts as a proxy (as your folder structure says correctly). However, why don't we re-use the OrderUpdater also in the proxy strategy and re-implement a new one?
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #1918 +/- ##
==========================================
+ Coverage 71.12% 72.69% +1.56%
==========================================
Files 149 155 +6
Lines 14309 14790 +481
Branches 1892 1919 +27
==========================================
+ Hits 10177 10751 +574
+ Misses 3593 3529 -64
+ Partials 539 510 -29 🚀 New features to boost your workflow:
|
hannesdiedrich
left a comment
hannesdiedrich
left a comment
There was a problem hiding this comment.
Thanks a lot for the explanations on- and offline.
Left a few follow up questions. Will LGTM once answered.
| "anonymousRecipient": [], | ||
| } | ||
| try: | ||
| with ws_connect( |
There was a problem hiding this comment.
Why do we use websocket and not a simple HTTP request for sending?
For _listener I get it, but not here.
There was a problem hiding this comment.
Mostly to reuse the same protocol when sending and receiving data. However I think that you are correct and it is much more reliable to use HTTP for sending data. Fixing this
| @property | ||
| def now(self) -> DateTime: | ||
| """Return the current time.""" | ||
| return now("UTC") |
There was a problem hiding this comment.
I understand. If this is the only time we set the time-zone that this is fine. If not, we could have a constant on package level.
| base_url = _CG_GATEWAY_URL.rstrip("/") | ||
| self._ws_url = base_url + _CG_WS_PATH | ||
| self._http_url = base_url + _CG_HTTP_MESSAGES_PATH |
There was a problem hiding this comment.
NIT: We could use urllib.parse.urljoin here
2 participants
Reason for the proposed changes
Please describe what we want to achieve and why.
Proposed changes
INTEGRATION_TESTS_BRANCH=master
GSY_FRAMEWORK_BRANCH=master
SCM_ENGINE_BRANCH=master