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

Handle unsupported python/ruby versions in a better way #175

Closed
david-gang opened this issue Aug 11, 2024 · 9 comments
Closed

Handle unsupported python/ruby versions in a better way #175

david-gang opened this issue Aug 11, 2024 · 9 comments
Labels
bug Something isn't working

Comments

@david-gang
Copy link

Describe the bug you encountered:

Pyroscope-rs let's you install on unsupported python and ruby versions and then swallows the fact that it cannot upload the profiles silently

What did you expect to happen instead?

  1. Document explicitely which python/ruby versions you support
  2. Add these requirements in the python and ruby libraries so that it will throw an error in installation time
  3. If you see that you cannot upload the profiles please write a warning

Look here for many bugs:

#168
#173
#80

My use case is that the ruby sdk works in 3.1.4 but not in 3.1.5 and i get absolutely no error message. It is the same as #173 If i would have get an error in the installation time i would have considered waiting with the upgrade to 3.1.5 .

...

How did you install pyroscope-rs?
with bundle


pyroscope-rs version and environment

@david-gang david-gang added the bug Something isn't working label Aug 11, 2024
@david-gang
Copy link
Author

david-gang commented Aug 16, 2024

I could take care of restricting pyroscope-rs for the python versions 3.7 to 3.11 and look how to do this in ruby. The question is if PRS are welcome. The repo looks a bit quiet in the last months.

@korniltsev
Copy link
Collaborator

We don't need restrictions for old versions. We need support for new versions.

@david-gang
Copy link
Author

david-gang commented Aug 16, 2024

That's a valid point but there will be always a gap between when a new python version is released and this project supports this new version. The same goes for ruby. From my experience i installed python 3.12 and then got an error while using the sdk forcing me to use ebpf.
In ruby it was even more weird. My colleague upgraded the ruby version from 3.1.4 to 3.1.6 and i got no warning or error message. The profiles just disappeared silently.
So the best would be to get a failure in the installation phase instead of getting the error anyway later
So either :

  1. This project starts to support ruby and python versions while there are still in the rc phase.
  2. They have in the requirements file (either gemfile or setup.py) which python versions they support.
  3. You have a different dependency which supports newer versions of python/ ruby transparently

Of all options i would prefer 3 but i am not sure this is possible, afterwards 1.
I agree the 2 is not great but it is better than the current situation.

WDYT?

@korniltsev
Copy link
Collaborator

I think we don't need restrictions for old versions. We need support for new versions.

@david-gang
Copy link
Author

david-gang commented Aug 16, 2024

That's great. I am waiting for python 3.12 support but in 0.512 it is not supported.
There is nothing which hinders you to install pyroscope-rs on an unsupported version including python 3.12.
And let's say you make a new release which supports python 3.12 apparently won't support python >= 3.13

Regarding the limitation this is not my invention. Here are two high starred libraries which do the same thing.

https://github.com/robotframework/SeleniumLibrary/blob/master/setup.py#L44
https://github.com/tensorflow/privacy/blob/master/setup.py#L48

Anyway this is your project so if you disagree, then just close the issue. Just bear in mind that the current lack of support for newer python and ruby versions plus the fact that they don't fail in the installation phase cause lots of agony.

Thanks,
David

@korniltsev
Copy link
Collaborator

korniltsev commented Aug 16, 2024

cause lots of agony

I understand and agree.

@david-gang
Copy link
Author

david-gang commented Aug 18, 2024

@korniltsev may you please share your plans how you are going to achieve support for new versions in a transparent way. This is especially hard in the case of pyroscope-rs as there is no official api of the languages themselve. This is not like go which has pprof as part of its sdk

@korniltsev
Copy link
Collaborator

I don't know for sure

@korniltsev
Copy link
Collaborator

I am not going to work on this in the near future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants