-
Notifications
You must be signed in to change notification settings - Fork 0
[DEV-3152] Add EAS Python SDK methods for running opendss export #30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[DEV-3152] Add EAS Python SDK methods for running opendss export #30
Conversation
|
Task linked: DEV-3152 Add EAS python sdk methods |
493146a to
3fcfba2
Compare
charlta
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very quick once over, so just adding as comments. Will leave to others that can test it to give approval etc.
src/zepben/eas/client/eas_client.py
Outdated
| response = await response.text() | ||
| return response | ||
|
|
||
| def get_paged_opendss_models(self, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is very much a nipick, feel free to ignore:
the format of this signature doesnt match init
ie: init has self on a newline, and all the args are indented only one spacing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed in 62c36bc
src/zepben/eas/client/eas_client.py
Outdated
| """ | ||
| return get_event_loop().run_until_complete(self.async_get_paged_opendss_models(limit, offset, query_filter, query_sort)) | ||
|
|
||
| async def async_get_paged_opendss_models(self, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: as above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed in 62c36bc
src/zepben/eas/client/eas_client.py
Outdated
| response = await response.text() | ||
| return response | ||
|
|
||
| def get_opendss_model_download_url(self, id: int): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: id is shadowing a built-in, perhaps run_id?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed in 62c36bc
src/zepben/eas/client/eas_client.py
Outdated
| """ | ||
| return get_event_loop().run_until_complete(self.async_get_opendss_model_download_url(id)) | ||
|
|
||
| async def async_get_opendss_model_download_url(self, id: int): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: id is shadowing a built-in, perhaps run_id?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed in 62c36bc
src/zepben/eas/client/opendss.py
Outdated
| @@ -0,0 +1,55 @@ | |||
| # Copyright 2020 Zeppelin Bend Pty Ltd | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copyright date, 2025?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops. fixed in 2dfab29
| } | ||
| } | ||
| """ | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: missing blank line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed in 62c36bc
| assert actual_body['variables'] == { } | ||
|
|
||
| return Response(json.dumps({"result": "success"}), status=200, content_type="application/json") | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: missing blank line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed in 62c36bc
| res = eas_client.get_paged_opendss_models() | ||
| httpserver.check_assertions() | ||
| assert res == {"result": "success"} | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: missing blank line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed in 62c36bc
| res = eas_client.get_opendss_model_download_url(1) | ||
| httpserver.check_assertions() | ||
| assert res == "https://example.com/download/1" | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: missing blank line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed in 62c36bc
| response = await response.text() | ||
| return response | ||
|
|
||
| def run_opendss_export(self, config: OpenDssConfig): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not a job for now and more of an observation then anything, but it almost feels like this file needs to be broken up. at >1000 lines its a little unwieldy - and has, in my opinion, alot of logic related to different components.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah would be nice if this was a client where you could see what it can do 😄
Signed-off-by: clydeu <clyde.uyenghua@zepben.com>
Signed-off-by: clydeu <clyde.uyenghua@zepben.com>
Signed-off-by: clydeu <clyde.uyenghua@zepben.com>
Signed-off-by: clydeu <clyde.uyenghua@zepben.com>
Signed-off-by: clydeu <clyde.uyenghua@zepben.com>
Signed-off-by: clydeu <clyde.uyenghua@zepben.com>
Signed-off-by: clydeu <clyde.uyenghua@zepben.com>
Signed-off-by: clydeu <clyde.uyenghua@zepben.com>
Signed-off-by: clydeu <clyde.uyenghua@zepben.com>
Signed-off-by: clydeu <clyde.uyenghua@zepben.com>
Signed-off-by: clydeu <clyde.uyenghua@zepben.com>
Signed-off-by: clydeu <clyde.uyenghua@zepben.com>
Signed-off-by: Roberto Marquez <roberto.marquez@zepben.com>
f1f4c14 to
2b14e65
Compare
roberto-marquez
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need this for a deployment so it will get merged now. If anyone cares about the additional changes to this PR they can open a ticket in Clickup
Description
Add client methods for running opendss export, listing the runs and getting a download url.
Associated tasks
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
Documentation
Breaking Changes
No breaking change.