-
Notifications
You must be signed in to change notification settings - Fork 4
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
Added port creation and floating IP logic. #11
Conversation
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.
A few comments!
As a more general note - you'll likely want to deploy this locally and test with updated UI code. In my experience, these method signatures can fluctuate as the UI changes, and it's better to test them together rather than have multiple incremental updates to the SDK. |
Link to the esi-ui PR - CCI-MOC/esi-ui#27 |
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.
Added two inline comments!
esi/lib/networks.py
Outdated
:return: The created port object | ||
""" | ||
if not name: | ||
name = 'esi-port-{0}'.format(network.name) |
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.
Can you name the port the same way we do here? https://github.com/CCI-MOC/esisdk/blob/main/esi/lib/nodes.py#L212
It actually probably makes sense to create a new port_name
function, and just call it both here and in the place that I linked.
@tzumainn Look into this and UI code both are updated. |
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.
Tested this with the updated UI - works well!
Description: