-
Notifications
You must be signed in to change notification settings - Fork 53
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: Add ability to install plugins #69
Conversation
Do you think this is something we could add a test for, to verify that plugins are really installed @connortann? 🙂 |
Oh nice, I didn't realise you had a test suite in GitHub workflows, I didn't think to look there. Certainly, I'll have a go! I'll need to think about to verify which plugins are installed; will do some experimentation. |
That release is yanked, it causes issues with other packages that require poetry. |
The Poetry 1.2.0a2 release was yanked? I guess that won't impact the feature, but maybe we should use 1.2.0 for the docs example, even though it's not released. Cheers for adding the test - that looks good to me @connortann 🙂 Would you like to add docs here as well? If you prefer I can add docs myself in a separate PR, just let me know 👍 |
Sure, I'll suggest adding something to the README. I also want to double-check the shell script I've added, so will set this PR as "WIP" for now. Also, I noticed that the ShellCheck CI linter doesn't check the shell script within EDIT: another option could be to use a CI job to temporarily extract the shell scripts to a .sh file, before running ShellCheck |
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
I'd love to do this, but as far as I know (I hope I'm wrong), there's no way to bundle a Also wish there was something like this just with shellcheck and action.yml files 😛 If you can work out a way of getting this to work, I'd be very happy to accept it. I usually throw the code into a shell script, lint it, then copy it back. |
I've attempted the migration to a EDIT: rebased this PR on top of main and fixed the conflicts. Nice having the ShellCheck linter working, it helped me avoid a few bash style issues. |
plugins: poetry-version-plugin, poetry-export-plugin | ||
- run: | | ||
source .github/scripts/assert.sh | ||
poetry plugin show |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The command poetry plugin show
seems to be failing on CI, with what looks to be possibly an issue with Poetry itself? Any help debugging would be appreciated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possibly related issue: python-poetry/poetry#4290
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree this seems related. Do you think we'll need to wait for the next release candidate version to proceed with this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potentially, although that would be a shame. As an alternative, do you think it would be possible to "pip install" the plugins?
I think if Poetry is "pip-installed", then the plugins can also be pip-installed. Not sure though how this works when Poetry is installed with install-poetry.py
; perhaps by activating the venv and then using pip?
This seems a little non-standard; but, all the more reason for it to be implemented in this action so others don't have to deal with the same complexities.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively, we could comment out that part of the test for now, and implement the plugins feature, with a note in the README that the plugins system is still pre-release and subject to errors. I think our code is ready to go, for example I would use it in my project to install the dynamic-versioning
plugin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've never really done a proper deep dive in the installation script, but on first glance it seems to just create a venv in an os-specific path (here-ish), so we certainly would have a pip-executable we could use to install things into that venv with.
Are Poetry plugins just added to the poetry venv anyway? In that case this seems like it would work 🤔
Closing this for now @connortann, but feel free to reopen. Think most of the blockers are resolved now. |
Closes #53
Adds ability to install plugins with:
Might be worth adding a check that the Poetry version is at least 1.2 before trying to install plugins? I also note that version is not released yet, so possibly the syntax may change in future.