search

gnocchixyz / gnocchi

Timeseries database
Apache License 2.0
299 stars 85 forks source link

Use pbr rather than direct easy install #1312

Closed jokke-ilujo closed 1 year ago

jokke-ilujo commented 1 year ago

Adds 'pbr' as dependency.

Adds '.venv' and 'build/' to gitignore. Introduces wgsi entrypoint gnocchi-app. Removes direct calls to setuptools easy install in favour of letting pbr to handle the install.

Closes Issue: 1304

jokke-ilujo commented 1 year ago

Looks like we need to pin the setuptools <68.0.0 on stable/4.4 first to unblock the upgrade job. After that this PR should do the trick for path forward.

tobias-urdin commented 1 year ago

I don't have an strong opinion on this, pbr was removed in the past [1]. What would be needed here is clearing with the distribution packagers to see what they prefer.

Since Gnocchi is still heavily integrated into the OpenStack ecosystem I suspect it would not be an issue but I wouldn't want to break their world either.

If anybody is faster to clear up that detail we could just merge this and have it land in next release.

[1] https://github.com/gnocchixyz/gnocchi/commit/3f8a22a51bec3a60709bcbf26648c4fd6a66d2b8

jokke-ilujo commented 1 year ago

I don't have an strong opinion on this, pbr was removed in the past [1]. What would be needed here is clearing with the distribution packagers to see what they prefer.

Since Gnocchi is still heavily integrated into the OpenStack ecosystem I suspect it would not be an issue but I wouldn't want to break their world either.

If anybody is faster to clear up that detail we could just merge this and have it land in next release.

[1] 3f8a22a

That explains the references to pbr but no evidence of it being in use. :D

Totally understand the concern for packagers quality of life. I'd like to argue at least pbr is fairly actively maintained unlike evidently the special sauce why it was dropped in the first place. Like you mentioned due to the close relationship pbr is also well understood from the OpenStack side and could hopefully ease the pain next time something breaks.

Let me know if you want me to resolve those merge conflicts and address any other possible concerns and I'll try to look after this.

tobias-urdin commented 1 year ago

I don't have an strong opinion on this, pbr was removed in the past [1]. What would be needed here is clearing with the distribution packagers to see what they prefer. Since Gnocchi is still heavily integrated into the OpenStack ecosystem I suspect it would not be an issue but I wouldn't want to break their world either. If anybody is faster to clear up that detail we could just merge this and have it land in next release. [1] 3f8a22a

That explains the references to pbr but no evidence of it being in use. :D

Totally understand the concern for packagers quality of life. I'd like to argue at least pbr is fairly actively maintained unlike evidently the special sauce why it was dropped in the first place. Like you mentioned due to the close relationship pbr is also well understood from the OpenStack side and could hopefully ease the pain next time something breaks.

Let me know if you want me to resolve those merge conflicts and address any other possible concerns and I'll try to look after this.

I wholeheartedly agree with you, perhaps we can get some feedback from @amoralej and @javacruft here.

javacruft commented 1 year ago

@tobias-urdin as stated PBR is pretty well understood and widely used across the OpenStack ecosystem so no issue from Ubuntu packaging team perspective.

amoralej commented 1 year ago

+1 to move back to pbr. I think the idea of moving out of was related to making gnocchi less openstack-centric but IMO pbr works well, is well maintained and widely used. With my RDO packager hat, I prefer gnocchi to be consistent with the OpenStack ecosystem (FTR, moving to setuptools_scm was a problem for us in the past).

tobias-urdin commented 1 year ago

@jokke-ilujo I think we have atleast an majority consensus here, I've also contacted @thomasgoirand for Debian packaging feedback.

I have a question thought, can we keep the wsgi_script as gnocchi-api to prevent changing any paths, it should be possible for that to coexist with the gnocchi-api command right without us having to change to gnocchi-app like this change does?

Also to be explicitly clear for anybody reading this, we would not be backporting the addition of PBR.

amoralej commented 1 year ago

I have a question thought, can we keep the wsgi_script as gnocchi-api to prevent changing any paths, it should be possible for that to coexist with the gnocchi-api command right without us having to change to gnocchi-app like this change does?

Yes, please, we should maintain backwards-compatibility, at least for deprecation notice and adapt tooling (or simply leave it as-is).

jokke-ilujo commented 1 year ago

I'll try to figure it out. It isn't at least obvious without digging ourselves back into the very same easy-install hole we're trying to climb out of. If you define same entry-point as console_script and wsgi_script for setuptools, only the console_script gets generated.

stephenfin commented 1 year ago

I don't think this is a good move and I struggle to see what problem it resolves.

I'll try to figure it out. It isn't at least obvious without digging ourselves back into the very same easy-install hole we're trying to climb out of. If you define same entry-point as console_script and wsgi_script for setuptools, only the console_script gets generated.

This is my main concern. Ultimately pbr is relying on deprecated APIs that are likely to be removed at some point in the future. From what I can tell, it seems the console_scripts stuff is handled by pip nowadays rather than setuptools. If we want to support wsgi_scripts in the new world, we're going to have to talk to the Python Packaging Authority (PyPA) folks (Donald Stufft et al.). Absent that, I suspect we will need to resort to the legacy script stuff and all the problems that entails :disappointed: Using pbr doesn't magically fix anything though.

stephenfin commented 1 year ago

Let's see what appetite is there for this https://discuss.python.org/t/adding-support-for-wsgi-scripts-entrypoint/30905

jokke-ilujo commented 1 year ago

Hmm-m the docs failures really aren't that clear. Any ideas?

tobias-urdin commented 1 year ago

After the feedback from @stephenfin and the patch https://github.com/gnocchixyz/gnocchi/pull/1327 (and https://github.com/gnocchixyz/gnocchi/pull/1325 though unrelated to PBR) I tend to lean more to that then to migrate to PBR.

Waiting for feedback from all of you before moving forward with that, thanks!

mrunge commented 1 year ago

I really like @stephenfin patches

paramite commented 1 year ago

+1

mrunge commented 1 year ago

@tobias-urdin what would be your preference to proceed here? This unfortunately blocks OpenStack Telemetry upstream gates in all branches

tobias-urdin commented 1 year ago

@mrunge do you mean supporting newer setuptools? that should be fixed by this https://github.com/gnocchixyz/gnocchi/pull/1314/commits/b52f7414d595e8d765fde50992f2776a77b98de6 that is also backported to stable branches just not released yet.

mrunge commented 1 year ago

oops, I certainly missed https://github.com/gnocchixyz/gnocchi/commit/b52f7414d595e8d765fde50992f2776a77b98de6 . I just stumbled upon the failures in telemetry ci again.

tobias-urdin commented 1 year ago

Closing in favor of merged https://github.com/gnocchixyz/gnocchi/pull/1327