grafana-toolbox / grafana-wtf

Grep through all Grafana entities in the spirit of git-wtf.
GNU Affero General Public License v3.0
138 stars 12 forks source link

Problem with grafana-client 4.0.0: `TypeError: expected string or bytes-like object` #126

Closed amotl closed 3 months ago

amotl commented 3 months ago

Problem

E       TypeError: expected string or bytes-like object

-- https://github.com/panodata/grafana-wtf/actions/runs/8482124688/job/23240743427?pr=125#step:5:510

References

Thoughts

Maybe Niquests returns bytes, where Requests previously returned text?

/cc @Ousret

amotl commented 3 months ago

In response to this hiccup, we just yanked the release '4.0.0' from PyPI again. It needs more investigation.

amotl commented 3 months ago

There is also:

AttributeError("'Version' object has no attribute '_version'")
Ousret commented 3 months ago

Weird. I don't follow what is happening. All seems related to self.api.version not yielding something correct.

import grafana_client
from verlib2 import Version

if __name__ == "__main__":

    gc = grafana_client.GrafanaApi(host="play.grafana.com")

    print(gc.version)
    print(Version(gc.version))

this work flawlessly, can you know what is returned by :

path = "/health"
return self.client.GET(path)

On your setup?

amotl commented 3 months ago

What about this guy?

ERROR    grafana_wtf.core:core.py:317 Request to /health endpoint failed: send() got an unexpected keyword argument 'on_post_connection'
        # Send the request
>       r = adapter.send(request, **kwargs)
E       TypeError: send() got an unexpected keyword argument 'on_post_connection'

-- https://github.com/panodata/grafana-wtf/actions/runs/8482124688/job/23240743427?pr=125#step:5:233

amotl commented 3 months ago
import grafana_client
from verlib2 import Version

if __name__ == "__main__":

    gc = grafana_client.GrafanaApi(host="play.grafana.com")

    print(gc.version)
    print(Version(gc.version))

    path = "/health"
    response = gc.client.GET(path)
    print(response, type(response))
$ python testme.py
11.1.0-68793
11.1.0.post68793
{'commit': 'df39fc54a6a7586e6da55270bdd4ca9d4d687e2b', 'database': 'ok', 'enterpriseCommit': '6543ac51c17e69bd6ac3c8ad6bbdcf3d3037802c', 'version': '11.1.0-68793'} <class 'dict'>
Ousret commented 3 months ago

The faulty part is this for the "on_post_connection" error https://github.com/panodata/grafana-wtf/blob/6dd8b91a4372fd2f0e2681f901b5724e2c084755/grafana_wtf/core.py#L110 you need to patch it to inject a proper HTTPAdapter from niquests.

Ousret commented 3 months ago

I guess, the whole thing is linked to the hybrid niquests/requests adapters. so, after cleaning this up, all will work fine.

And you don't need to do pool_connections=100, pool_maxsize=100, max_retries=5, in the HTTPAdapter anymore, it can be set within the Session constructor,

so instead of

adapter = requests.adapters.HTTPAdapter(pool_connections=100, pool_maxsize=100, max_retries=5, pool_block=True)
self.grafana.client.s.mount("http://", adapter)
self.grafana.client.s.mount("https://", adapter)

return self

do

self.grafana.client.s = Session(pool_connections=100, pool_maxsize=100, retries=5)

return self
amotl commented 3 months ago

So that's clearly my fault on grafana-wtf, where I wrongly verified success beforehand. So, effectively, the release would not need to have been yanked, so we un-yanked it again. Enjoy, and thanks for your support.

Ousret commented 3 months ago

No worries. I understand, Look at the previous comment, for guidance on how you can increase the pool scale properly without handling HTTPAdapter anymore.

amotl commented 3 months ago

Thanks. Orthogonal to that: Maybe Niquests can come up with a better error message, and, more importantly, fail early, when it detects that it is processing on behalf of a corresponding object instance from requests instead of niquests? It can not be expected the other way round, but Niquests could do it we guess?

Ousret commented 3 months ago

Yes, you are right. Niquests will handle those cases more gracefully, I will do this asap. Side note, grafana client is marked as "universal wheel" py2, py3 when it should not.

This section should be removed in the setup.cfg

[bdist_wheel]
universal = true
amotl commented 3 months ago

Thank you very much, is it a leftover from Python 2 times? We will run a 4.0.1 fixup release.

Ousret commented 3 months ago

I think so yes, a remnant from the past versions.

amotl commented 3 months ago

Away with it.