eldarion-gondor / gondor

Gondor client command-line tool
1 stars 0 forks source link

ignore instance flag when inspecting/modifying service environment variables #2

Open jacobwegner opened 8 years ago

jacobwegner commented 8 years ago

Currently, a service level environment variable cannot be set until an instance is mapped to a branch in gondor.yml:

$ g3a services env <service-name>
ERROR: unable to map "master" to an instance. Please provide --instance or map it to an instance in gondor.yml.

g3a services env <service-name> --instance=primary
Incorrect Usage.

NAME:
   services env -

USAGE:
   command services env [arguments...]

A service can only be attached to one instance, so the env command should not require the --instance flag, and should not set the instance based on the current mapping in gondor.yml.

brosner commented 8 years ago

This is trickier than it may seem. While it is true the service name can only be in reference to one instance, the client doesn't see it that way. To get the service it has to start at the site then select the instance to do the final lookup of the service name. So at some point the instance needs to be defined. Like instances env the --instance flag was overlooked. I think the only fix here is to add it. One other possibility is during auto-complete we search across all services under a site and auto-complete the instance too. We can track that separately though.

jacobwegner commented 8 years ago

@brosner: Understood about the lookup order.

From looking at the current call chain on the client, I thought maybe the services could just be derived from the return of sites request:

g3a services env <service_name> --log-http
# GET /v2/resource_groups/find/?name=<resource_group> HTTP/1.1
# GET /v2/sites/find/?name=<site_name>&resource_group=<resource_group_url> HTTP/1.1
{
    "url": <site_url>,
    "name": <site_name>,
    "resource_group":{
        "id": <id>,
        "name": <resource_group>,
        "url": <resource_group_url>,
        "users": [
            ...
        ]
    },
    "instances": [
        {
            "url": <instance_url>,
            "label": <instance_name>
            "kind": <instance_kind>,
            "state": <instance_state>,
            "web_url": <instance_web_url>,
            "services": [
                {
                    "url": <service_url>,
                    "name": <servie_name>,
                    "kind": <service_kind>,
                    "replicas": <replica_count>,
                    "state": <service_state>
                }
            ]
        }
    ]
}
brosner commented 8 years ago

This is a very valid point that I didn't even consider and one I specifically designed for in the API. Good catch.