bug fixing and a couple updates

[IrSent]

• prometheus-client==0.6.0
• a couple of comments
• a couple of docstring fixes
• tags = kwargs.get('tags') or {} workaround:
  At some point tags key in kwargs gets None and I receive TypeError: 'NoneType' object does not support item assignment on the line: tags['unit'] = config.unit

[bloodearnest]

Hey, thanks for this

I suspect this change will break support for prometheus_client <0.6.0?

[IrSent]

yes, is there a need to support previous versions for a software that is still in 0. version? :)
Because they are changing internals constantly I don't know what to do with previous versions.

[bloodearnest]

Yes I think there is the need to support those older versions, as they are deployed in production. The fact it's 0.x doesn't change anything IMO - prometheus_client has been 0.x for a long time.

It's not difficult to support older versions in this case:

try:
    from prometheus_client.values import MultiProcessValue
except ImportError:
    from prometheus_client.core import _MultiProcessValue as MultiProcessValue

[bloodearnest]

Note the comment at the top of setup.py - it is generated from setup.cfg, so that's where this change needs to be made, and run 'make setup.py' to update setup.py.

[IrSent]

thanks, will take a look

[IrSent]

I've modified the setup.cfg to include 0.6.0.

[bloodearnest]

Hey, thanks for the fixes.

I would like to split the work supporting prometheus_client 0.6.0 out into a separate PR, if that's OK? And as discussed already, we need to maintain older prometheus_client lib compat, and I'd like to keep the != statements, as there are bad bugs in those releases that break multiprocss metrics.

I'd be happy to have a later PR to drop support for older prometheus_client, but we'd need to do a release to allow folks to transition.

Thanks!

[bloodearnest]

This pull request fixes 2 alerts when merging f3d3fdc into a990ef9 - view on LGTM.com

fixed alerts:

• 1 for Signature mismatch in overriding method
• 1 for Missing call to __init__ during object initialization

---

Comment posted by LGTM.com

[IrSent]

@bloodearnest It seems that prometheus-client with version 0.2.0 doesn't collect metrics correctly to generate_latest on the endpoint (https://travis-ci.org/canonical-ols/talisker/jobs/538016434#L571) Do we really need that 0.2.0 support? Can we skip this test for this build if the functionality itself doesn't work?

[bloodearnest]

Hmm, those tests used to pass under 0.2.0 before. But yes, happy to move the lower bound >=0.3 if make things easier for us.

…

On Tue, 28 May 2019 at 12:35, Michael ***@***.***> wrote: @bloodearnest <https://github.com/bloodearnest> It seems that prometheus-client with version 0.2.0 doesn't collect metrics correctly to generate_latest on the endpoint ( https://travis-ci.org/canonical-ols/talisker/jobs/538016434#L571) Do we really need that 0.2.0 support? Can we skip this test for this build if the functionality itself doesn't work? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#455?email_source=notifications&email_token=AAAAIERVTW33IHXPAMRWARLPXUKIZA5CNFSM4HPQBM7KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWL2W7I#issuecomment-496479101>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAAIES2QEVGXA24OHXHJWTPXUKIZANCNFSM4HPQBM7A> .

-- Simon

[bloodearnest]

Why was sphinx downgraded?

[IrSent]

Requires: Python >=3.5

[IrSent]

https://pypi.org/project/Sphinx/

[bloodearnest]

Hi, I think this is good to land, apart from the Sphinx downgrade?

Edit: ignore me, I missed the test failures. Feel free to update the minimum prometheus_client to 0.3.0 if that helps.

[IrSent]

Hi, I think this is good to land, apart from the Sphinx downgrade?

You want to avoid downgrading Sphinx version? I don't think I've understood your question, sorry.

[IrSent]

BTW, prometheus-client >=0.3.0 doesn't solve the empty metrics response in test.

[bloodearnest]

Hi, I think this is good to land, apart from the Sphinx downgrade?

You want to avoid downgrading Sphinx version? I don't think I've understood your question, sorry.

Yes. Sphinx is only used in the default development environment, and docs tox environment, both of which are python 3. Only talisker itself needs to support python2, the dev/build tooling is py3 only.

[bloodearnest]

BTW, prometheus-client >=0.3.0 doesn't solve the empty metrics response in test.

Ok - does 0.4.2+ solve it? If so, lets make it >=0.4.2,<=0.6.0?

I don't mind deprecating oldest versions, in general, I just don't like pinning to single version for libraries like Talisker, as it forces people to upgrade things all at once, and can get in the way of urgent bug fix releases. Especially with sth like prometheus, which is fairly crucial operationally. The prometheus_client project does not have a good record regards b/w compat releases, so we try move carefully there. And the multiprocess metrics stuff is very unpleasant, and reaches into prometheus_client by necessity, so it more likely to be break on new releases.

I just notices we still have some production services on 0.3.x, I'll have to fix that :)