meraki / dashboard-api-python

Official Dashboard API library (SDK) for Python
MIT License
287 stars 149 forks source link

passing pagination (total_pages) params to getOrganizationApplianceTrafficShapingVpnExclusionsByNetwork results in error #247

Closed zabrewer closed 1 month ago

zabrewer commented 5 months ago

Describe how you confirmed the issue is with the library, and not with the API itself, or a server-side issue of some other kind. Verified on two different systems with different code and did other testing to make sure it was library related

Python version installed Python 3.8.10

Meraki library version installed 1.42.0

Have you reproduced the issue with the latest version of this library? And with the latest version of Python? latest version of the library, yes. Latest version of python, no (3.12 is out - 3.8 is common and well supported)

OS Platform OSX 14.3 and Windows 11

Describe the bug If you pass in total_pages = -1 or total_pages = 'all' you get the following error above (key of pageStartAt not found): Error:

Traceback (most recent call last):
  File "test.py", line 45, in <module>
    exclusions = session.get_orgtrafficshaping(org_id='708753991357431888')
  File "test.py", line 37, in get_orgtrafficshaping
    return self.api_session.appliance.getOrganizationApplianceTrafficShapingVpnExclusionsByNetwork(organizationId=org_id, total_pages='all')
  File "OMITTEDi/appliance.py", line 2433, in getOrganizationApplianceTrafficShapingVpnExclusionsByNetwork
    return self._session.get_pages(metadata, resource, params, total_pages, direction)
  File "OMITTED/meraki/rest_session.py", line 452, in _get_pages_legacy
    start = response.json()['pageStartAt']
KeyError: 'pageStartAt'

relevant library code is in https://github.com/meraki/dashboard-api-python/blob/main/meraki/api/appliance.py on line 2401

How can we replicate the problem you're reporting? 1) setup session using Meraki Python SDK as usual 2) call getOrganizationApplianceTrafficShapingVpnExclusionsByNetwork and pass in total_pages = -1 or total_pages = 'all'

Expected behavior should get a 200 OK and data returned

TKIPisalegacycipher commented 4 months ago

Thanks for reporting this @zabrewer -- I notice it's using get_pages_legacy instead of the newer iterator. I wonder if it happens using the newer iterator, as well?

ollipa commented 2 months ago

@TKIPisalegacycipher, we are also hitting this but with getOrganizationWirelessSsidsStatusesByDevice when total_pages="all" is passed. This doesn't seem to happen always so I can't reproduce the bug reliably.

File "meraki/aio/rest_session.py", line 473, in _get_pages_legacy
    start = json_response["pageStartAt"]
timoturkki commented 2 months ago

@TKIPisalegacycipher we've had many cases of this errors happening today and yesterday (27 and 28th of June) affecting multiple different users. All cases seems to be related to getOrganizationWirelessSsidsStatusesByDevice -operation.

The error seems to happen when there are multiple pages to fetch. Also seems that the network requests can be quite slow, sometimes taking more than 20s to complete.

TKIPisalegacycipher commented 1 month ago

It looks like the pagination tokens returned in the response headers for the VPN exclusions operation are not valid network IDs. It's possible that getOrganizationWirelessSsidsStatusesByDevice has a similar issue, but with serial numbers instead. You can check by using Postman to make the call with perPage=3, and checking the pagination links (rel=next, rel=prev, etc.). The pagination links should invoke query parameters startingAfter and/or endingBefore, and if the operation is paginated on network ID, then they should look like network IDs (e.g. L_xxx or N_xxx). Similarly, for pagination on serial number, then they should look like serial numbers (e.g. Q123-1111-1111).

This will need to first be fixed on the Meraki OAS side. If you haven't already, please open a case with Meraki support, with your own supporting evidence, for the operation that matters to you (or both, if you like). I would recommend a format like this, if you're working on the VPN exclusions operation:

The operation is paginated on network ID, but the tokens used for pagination, and shown in the response headers, are not properly formatted network IDs. Network IDs should take the form of "N_xxx" or "L_xxx". In these attached examples (attach your own real-world examples), the pagination links do not have any sign of a network ID.

Separately, it looks like the _get_pages_legacy function defined in rest_session.py might not be properly reading the pagination links from the operation response. If you are able to review this code to confirm and/or suggest a PR to fix it, that would be welcome and could be merged into main. However, this is not yet confirmed, so for now, I will close this issue based on the OAS-side problem not being something the library can fix.

I hope that Meraki will fix this quickly, and if anyone would like to open a separate discussion around the _get_pages_legacy method, that would be welcome.