meraki / dashboard-api-python

Official Dashboard API library (SDK) for Python
MIT License
293 stars 154 forks source link

Exception on organizations with disabled api access #134

Closed coreGreenberet closed 3 years ago

coreGreenberet commented 3 years ago

Hey,

there is currently an issue with organizations which don't have api access enabled. If you make requests to such an organization you will receive a 404 Not Found with this message:

{
    "errors": [
        "To make requests you must first enable API access via https://XXXX.meraki.com/o/XXXXX/manage/organization/edit"
    ]
}

which results in an exception on the sdk.

There are 2 problems here:

  1. it will break most scripts, because of the exception
  2. the developers can't see the issue until it occurs as there isn't a "apiEnabled" value in the result of getOrganizations

I can see ways to fix that issue:

  1. Catch that error in the sdk and log a warning without throwing an exception
    • Pro: Might be the quickest workaround
    • Con: This will only work for the python sdk. People which are using different programming languages will still have that issue
  2. the endpoint GET /organizations should return an "apiEnabled" attribute.
    • Pro: developers can see, if they can make api calls to that organization or not
    • Con: existing scripts will still break and existing and new scripts will have to be changed
  3. the endpoint GET /organizations should get an optional new parameter "showAPIDisabled" (default = false)
    • Pro:
      • it will work on existing scripts
      • it will work on any language
    • Con:
      • might confuse some developers in the beginning, because some organizations are not shown.
    • Additionally: this should probably be implemented together with point 2

I put this issue here up as a discussion base, as I'm unsure, if we should fix this python sdk wise or via meraki endpoint update.

I've also opened a support case on this.

coreGreenberet commented 3 years ago

In my opinion Point 3+2 would be the best solution here

TKIPisalegacycipher commented 3 years ago

I'm not sure this is something that needs to get built into the API or the SDK because it can be handled client side before running operations if you are expecting to run into 404s as a result of the organization not having API access enabled.

Separately, we try to avoid building conditionally shown params into any endpoint.

If I understand your proposal correctly then you would have every endpoint always return an additional parameter indicating whether or not API was enabled?

It seems like the amount of effort involved in checking that client side before performing operations is about the same as writing the error handling into the client for 404s (current behavior). I wonder how useful this would be.

Is there some more specific reason why this is a problem that can only be solved by the API or the SDK?

On Sun, Dec 27, 2020, 01:10 coreGreenberet notifications@github.com wrote:

In my opinion Point 3+2 would be the best solution here

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/meraki/dashboard-api-python/issues/134#issuecomment-751443522, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA5EUKTO3AC3IH6TZQEM6ELSW32ZXANCNFSM4VKUNV6A .

coreGreenberet commented 3 years ago

for example you have two organizations

A = api enabled B = api disabled

now you are making a call to /organizations

you get returned A & B

next step would probably be -> getOrganizationNetworks on A -> success next step would probably be -> getOrganizationNetworks on B -> error

I'm NOT saying to do it on EVERY endpoint. Just on getOrganizations therefore we could filter out all api enabled/disabled organizations and we don't have to worry about it anymore.

On the other hand: Why are api disabled organizations returned by /organizations at all?

The main reason, why I think it should be done in the API is because even all the samples are working like I've described above. So all will fail because of that as soon as you have an organization without api access bind to your user.

TKIPisalegacycipher commented 3 years ago

This is good feedback and worth considering for handling within the API itself, but we are also open to a community contribution to address this specifically for the SDK.

Otherwise, the SDK is technically operating as designed, so I will close this issue.