[skip ci] Define REST API handlers for listing VCHs#6155
Conversation
| for _, vch := range vchs { | ||
| var version *version.Build | ||
| var dockerHost string | ||
| var adminPortal string |
There was a problem hiding this comment.
you may want to declare them outside of the loop and reusing them if vchs potentially a big slice of vms
There was a problem hiding this comment.
If I declare them outside the loop, I need to re-zero them on each iteration.
There was a problem hiding this comment.
Just for curiosity I would take a look at the generated code, I suspect all these variable are allocated on stack.
There was a problem hiding this comment.
I'm not sure why it matters whether or not these three variables are allocated on the stack. Could you elaborate?
|
|
||
| if public := vchConfig.ExecutorConfig.Networks["public"]; public != nil { | ||
| if public_ip := public.Assigned.IP; public_ip != nil { | ||
| var docker_port string |
There was a problem hiding this comment.
docker_port := fmt.Sprintf("%d", opts.DefaultHTTPPort)
if !vchConfig.HostCertificate.IsNil() {
docker_port = fmt.Sprintf("%d", opts.DefaultTLSHTTPPort)
}
saves a declaration and branch :)
| model.UpgradeStatus = upgradeStatusMessage(ctx, vch, installerVer, version) | ||
| } | ||
|
|
||
| if adminPortal != "" { |
There was a problem hiding this comment.
the default values of AdminPortal and DockerHost is "" anyway so I think you could just assign them without checking
zjs
force-pushed
the
topic/vic-machine-service-impl-list
branch
from
399fdfc to
ec1fdfd
Compare
|
Are the tests you added here related to the VCH listing api handlers? |
zjs
force-pushed
the
topic/vic-machine-service-impl-list
branch
from
ec1fdfd to
6f60f17
Compare
Introduce a pair of handlers for listing information about VCHs, optionally scoped to a compute resource or datacenter. Include basic authentication and error handling, which should be improved on as development continues.
zjs
force-pushed
the
topic/vic-machine-service-impl-list
branch
from
6f60f17 to
f85af3b
Compare
5 participants
Introduce a pair of handlers for listing information about VCHs, optionally scoped to a compute resource or datacenter.
Include basic authentication and error handling, which should be improved on as development continues.
Resolves #6026.
Note: this change builds on #6154 and, once approved, I would like this to land as the fourth commit on
feature/vic-machine-service.