search

conductor-sdk / conductor-python

Conductor OSS SDK for Python programming language
Apache License 2.0
62 stars 29 forks source link

Latest changes to setup.cfg in release breaks imports #89

Closed aaj closed 2 years ago

aaj commented 2 years ago

https://github.com/conductor-sdk/conductor-python/commit/663474068bf6923be58276b49a99a245adc01ba7

I'm trying to pip install conductor-python and then import conductor, but I'm getting an No module named 'conductor'. Reverting to version 1.0.29 fixes it.

gardusig commented 2 years ago

Taking a look at it

gardusig commented 2 years ago

1.0.31 was removed from pypi by now, working on a fix and will publish it as 1.0.32 soon.

Looks like some misconfig from setup.cfg migration to setup.py

gardusig commented 2 years ago

Hey @aaj, I've just released 1.0.32 at pypi: https://pypi.org/project/conductor-python/1.0.32/

I think your issue should be solved by now :)

Special thanks for the quick feedback!

aaj commented 2 years ago

@gardusig awesome! Super quick turnaround, thanks for the heads up 🤗

matteius commented 2 years ago

Just curious @gardusig why to move from setup.cfg towards setup.py? When the setup.cfg is newer and seems to be the preferred direction that setuptools is heading in. I help maintain pipenv and any insights you could share would be useful to me. Thanks!

gardusig commented 2 years ago

Hey @matteius, I think this move was probably due to a bad solution aiming to fix an issue that I've faced: while creating a CI/CD pipeline, I wanted to change the setup to accept ENV vars previously set here on GitHub.

I'm not sure if it's possible to do that with only setup.cfg, the fastest way I found to solve this was to use a setup.py, which can read from os.environ and solves the problem without much effort :)

In the meantime to current latest release, I've just moved everything from .cfg to .py and forgot to add the actual package_dir parameter in the setup.

BTW appreciate any ideas on how to validate a release after it's published at pypi from GitHub Actions.

matteius commented 2 years ago

@gardusig Looks like you can reference environment variables in setup.cfg: https://stackoverflow.com/a/30679041

Also to validate a release after its published to pypi from Github Actions -- you might want to have a small sleep if there is a delay before it become available in pypi, but I would have a build step after the publish to pypi where you can either create a new virtualenv, or ensure that the package is no longer installed in your system environment -- then pip install from pypi the package and invoke python -c "import conductor" would do a sanity check that the module can be imported. The problem though is if you have already published to pypi and this check fails, now you have to yank it. Would be better to just build the package ahead of time and import that and see that it works before publishing now that I think more about it.