Fix service name collision or breakage #710
Conversation
|
Note that the uuid of service is created by FE, which is not ideal. I didn't look deeper to check if how BE process the services. @fruttasecca Feel free to chip in. |
| [ | ||
| is_service_definition_valid(service) and sname == service["name"] | ||
| for sname, service in services.items() | ||
| ] | ||
| [is_service_definition_valid(service) for _, service in services.items()] |
There was a problem hiding this comment.
I think it might disrupt some existing logic where it is/was assumed that the dictionary key and service name were interchangeable. Regardless of this, it might not be feasible because then users won't be able to resolve (i.e. dns) to a given service from within a step/kernel, or the logic around that needs to get more complicated.
Essentially we want users to be able to refer to service "redis" as redis, or at least that's what happens currently in the k8s branch, while prek8s it's something like service-<project uuid>-<pipeline uuid>.
There was a problem hiding this comment.
I think we want to keep service name uniqueness at the pipeline level
There was a problem hiding this comment.
Hmm, this means we actually don't allow users to have multiple instances of the same service in a pipeline?
I got it. Will change the PR.
There was a problem hiding this comment.
@fruttasecca BE changes have been removed.
|
Test summary
Run details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
iannbing
force-pushed
the
fix/change-service-name
branch
from
683457e
to
302c737
Compare
iannbing
force-pushed
the
fix/change-service-name
branch
from
302c737
to
18e6b11
Compare
Description
This PR fix the issue for service name collision and breakage.
If user attempts to rename a service with an existing name, the form will show a proper error message and disable all other fields until user gives a unique name.
Fixes: #375
Checklist
devinstead ofmaster.