search

juju / charm-helpers

Apache License 2.0
18 stars 127 forks source link

Recent move of contrib.python to fetch.python is a backward-incompatible module change #276

Open ryan-beisner opened 5 years ago

ryan-beisner commented 5 years ago

The following module change is a break in the module layout, making it backward-incompatible. OpenStack Charms are impacted and either have to change the way imports happen, or look to make this more compatible within the charm-helpers library.

https://github.com/juju/charm-helpers/pull/272

https://github.com/juju/charm-helpers/pull/272/commits/92ff061aa9dc6b0cc9bbfdba225c728feadabb44

thedac commented 5 years ago

This change will require touching 30-40 charm for OpenStack alone.

Please consider reverting 92ff061.

ryan-beisner commented 5 years ago

charmhelpers/contrib/python/ became charmhelpers/contrib/python.py, so there is no longer a charmhelpers/contrib/python/__init__.py file.

Our c-h sync tool expects an __init__.py file for charmhelpers.contrib.python.packages, and all charm helper library syncs are failing at the moment.

The shim, which I believe is pythonically-sound, could be made more structurally compatible by retaining the module dir structure of contrib.python as a module with an __init__.py file.

ryan-beisner commented 5 years ago

@johnsca Actually: after revisiting this, and the admittedly sub-optimal process of cargoing partial libraries into a repo via a tool that makes perhaps unsafe assumptions, I'm inclined to say: "It's not you. It's us." ;-)

Your initial shim, as already committed, should indeed be fine. It's simple and straight-forward. I think we should leave it in place, and proceed to improve the OpenStack Charms' charm-helper sync/consumption approach (and update our repos to deal with the move).

Thank you for your work on this., and the follow-up PR (https://github.com/juju/charm-helpers/pull/278/). I think it is safe to drop/abandon that follow-up PR.