Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[wip] Python 3 #762

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

[wip] Python 3 #762

wants to merge 3 commits into from

Conversation

erkie
Copy link

@erkie erkie commented Apr 9, 2022

It felt kinda weird that there was absolutely nothing in this repo about how to run Diamond using Python 3. Like many others I recently upgraded to Ubuntu 20.04 which does not ship with a fully formed python 2.7 environment so I gave a stab at making it work with Python 3.

This is very much a WIP but I got it running fully for our setup.

How to install for you:

pip3 install distro
pip3 install git+https://github.com/feederco/Diamond.git@d8429765009fdf115c85aca09a5f2c1b570f8078

Since platform.distro() was deprecated in 3.8 which setup.py relied on it now uses distro, which is a pip module that needs to be installed before running pip install diamond.

Todo:

  • Tests still not ported
  • So far only tested in production with MySQL collector and hostedgraphite and archive handler
  • This branch is all-out p3k, so we'd need to figure out how to interop these two languages, or tbh just make a fork, or keep python2 support in a branch, because python 2 seems pretty ded

References #396

Todo:
- Tests still not ported
- So far only tested MySQL collector and hostedgraphite handler
@erkie
Copy link
Author

erkie commented Apr 10, 2022

Btw, since this is running fine in our environment I won't be working more on this. Any additional PR's and help is greatly appreciated.

@shortdudey123
Copy link
Member

Can you split out the diamond relative to full from /import statements like at the top of the handler files to a separate PR? Those changes are non-breaking and I can merge it right away. Also include the three lines changed in bin/diamond.

@erkie
Copy link
Author

erkie commented Aug 31, 2022

@shortdudey123 I won't have time to do this anytime soon, sorry! Feel free to take over this branch or if any future readers see this please feel free to work off this branch 🙏

@kt97679
Copy link

kt97679 commented Nov 25, 2022

Thank you very much for the effort @erkie ! Based on your change I introduced more fixes to make diamond work in my environment: kt97679@8fee22d I would like to emphasize that I had to change config processing in the src/diamond/collector.py: kt97679@8fee22d#diff-65d20bb86255c9b2b68200bc4532b54b0c3fea580cf4ccaa0181c8259b2c2f6aL199-R208 I noticed that subsections using double square brackets are not parsed correctly so I switched from

[collectors]
...
[[default]]

to

[collectors]
...
[collectors.default]

Other changes are mostly automatic from the 2to3 script. In some places I had to add encode()/decode() logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants