apigee / apigeecli

This is a tool to interact with Apigee APIs. The tool lets you manage (create, del, get, list) environments, proxies, etc.
Apache License 2.0
51 stars 28 forks source link

Odd behavior querying app that does not exist #508

Closed DinoChiesa closed 1 month ago

DinoChiesa commented 1 month ago

It simply prints null :

$ apigeecli apps get --name "app-id-does-not-exist" --org "$PROJECT" 
null
srinandan commented 1 month ago

The API implements get by ID. I implemented a search by name. In the case where the name is not found, instead of returning an error, a null value was returned..

I have used the API's filter query param to perform a server side search by name. On the result, I combined it with a client side filter (gojsonq) to search for the app name. The client side library returned null.

I have fixed the bug in the PR below.

DinoChiesa commented 1 month ago

I think either

{}

or an Error, might be appropriate.

If I am correct, I think searching with other filters, might return an empty list, rather than an error.

Or rather, not an empty list, but a JSON object with a non-existent list. Eg normal is

{ "app" : [...] } 

But "not found" can be

{ }

wdyt?

kurtkanaskie commented 1 month ago

apps get --name=pingstatus-v1-app1 returns an array of same named apps [ {}, {} ]

with the fix it returns Error: unable to find developer app with name foo exit status 1

Is that what we want? @DinoChiesa

srinandan commented 1 month ago

The API response when a filter doesn't match anything (in this case, application name is not found) is:

HTTP 200 OK, followed by

{
  "totalSize": <number of apps>
}

I'm not in favor of constructing a JSON which is not returned by the API. I also don't see a point in showing the JSON returned by the API (because it is confusing).

As mentioned by @kurtkanaskie , the fix now returns:

Error: unable to find developer app with name foo
exit status 1

FWIW, other products appear to return an empty JSON {}

srinandan commented 1 month ago

On second thoughts, I propose this:

  1. Return the API response as-is. If Apigee changes the response, it is transparent to apigeecli.
  2. A filter not matching anything is not at error. So the current fix is incorrect. I have reverted it.
  3. I do print a warning statement in the case where the filter doesn't match anything. This warning can be suppressed.
kurtkanaskie commented 1 month ago

API returns not found

curlx https://apigee.googleapis.com/v1/organizations/$ORG/developers/kurtkanaskie@google.com/apps/notfound
{
  "error": {
    "code": 404,
    "message": "App named notfound does not exist under kurtkanaskie@google.com",
    "status": "NOT_FOUND",
    "details": [
      {
        "@type": "type.googleapis.com/google.rpc.PreconditionFailure",
        "violations": [
          {
            "type": "developer.service.AppDoesNotExist",
            "subject": "[2002:ab0:f5d7:0:b0:3e1:dfb1:f5b2]:4004:bccdi20-20020a056f0941d400b002c350fff3fd:9809:182966:194432970",
            "description": "App named notfound does not exist under kurtkanaskie@google.com"
          }
        ]
      },
      {
        "@type": "type.googleapis.com/google.rpc.RequestInfo",
        "requestId": "3189453791813552833"
      }
    ]
  }
}

This is an odd command in that it does not mimic an API call like these that return 404s curlx https://apigee.googleapis.com/v1/organizations/$ORG/apps/792f8686-128f-47f8-b3f4-d1df174011f8 curlx https://apigee.googleapis.com/v1/organizations/$ORG/developers/kurtkanaskie@google.com/apps/notfound

Rather it is more akin to a search request (list with a filter param), I think it should return an "empty" 200 representation of the resource [] like this API does. curlx https://apigee.googleapis.com/v1/organizations/$ORG/environments/local/targetservers []

Ah, looking at the API, I see what you are using now.

curlx https://apigee.googleapis.com/v1/organizations/apigeex-mint-kurt/apps?filter=appName=mint-v1-appdev-exco-int
{
  "app": [
    {
      "appId": "19665765-bf97-4d76-ac6a-8ce4eed5f646",
      "attributes": [
        {
          "name": "Description",
          "value": "pingstatus-v1 by +appdev on apigeex-mint-kurt-exco"
        },
        {
          "name": "DisplayName",
          "value": "mint-v1-appdev-exco-int"
        },
        {
          "name": "displayName",
          "value": "mint-v1-appdev-exco-int"
        }
      ],
      "createdAt": "1627309138550",
      "credentials": [
        {
          "apiProducts": [
            {
              "apiproduct": "pingstatus-mint-v1-product-1-test",
              "status": "approved"
            }
          ],
          "consumerKey": "eGZD0VeTn93AglAl6s9JTpalF5lypcLWNoQaKnw3Rube8nlf",
          "consumerSecret": "oEE7AZPvhinEUqm0sAHi8SIsRvRWSAybtB3VvjuYdvx0jOuAcfuj2ZmI8BpnPQkI",
          "expiresAt": "-1",
          "issuedAt": "1627391835253",
          "status": "approved"
        }
      ],
      "developerId": "8688af81-b11f-4ef0-810f-556098db1464",
      "lastModifiedAt": "1627508587837",
      "name": "mint-v1-appdev-exco-int",
      "status": "approved",
      "developerEmail": "kurtkanaskie+appdev@google.com"
    }
  ],
  "totalSize": 201
}

And for a not found app

curlx https://apigee.googleapis.com/v1/organizations/apigeex-mint-kurt/apps?filter=appName=notfound
{
  "totalSize": 201
}

I'm good with that.