-
Notifications
You must be signed in to change notification settings - Fork 40
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
CI failing on poetry-export #1473
Comments
We already do: https://github.com/mxcube/mxcubeweb/pull/1269/files#diff-63a9c44a44acf85fea213a857769990937107cf072831e1a26808cfde9d096b9R38. Where does it fail? This one https://github.com/mxcube/mxcubeweb/actions/runs/11549893904/job/32143790326?pr=1471? |
So, it should not show the warning. The issue has been reported: python-poetry/poetry-plugin-export#249. |
Here: On second thought I don't think its the plugin install that fails but that this will always fail on |
Yes, this one https://github.com/mxcube/mxcubeweb/actions/runs/11549893904/job/32143790326?pr=1471 failed due to poetry-export, but only after rebasing it to the newest changes in mxcubeweb |
Oh... Yeah... That might be the issue... I need to think about it. |
I think this was actually the issue, since we did not bump the version of
This one then failed because I did not re-run the pre-commits after the rebase, I tried it again with the needed changes for |
I can make a PR with the changes to |
#1472 was merged although the CI pipeline was not fully green. Nothing should be merged if the CI pipeline is not 100% green. I believe we should put the rules on the branches to prevent this from happening. Anyone knows how to do this on GitHub? |
Its already the case @fabcor-maxiv, I force merged because I thought it was the warning that was the issue and it was just a patch version bump. I wont do it on the future. However I'm a bit reluctant to do a manual rebase on dependabot PRs |
One Idea would also be to create a workflow that is specifically triggered when |
Ah ok, I see. Yes, I understand that having to manually modify the Dependabot pull requests would be annoying... :/ On the other hand: I added the auto-generated
Ah yes, that could be a solution, needs investigating... |
@fabcor-maxiv |
Sounds like a good idea to ask who is using the generated |
https://mxcubeweb.readthedocs.io/en/latest/dev/deployment.html#install-without-using-poetry But I wonder what "mess with
So you are using pinned dependencies for development and unpinned dependencies for production? Well, we have the same issue and I have been meaning to fix this in our deployment procedure, but I always get sidetracked... My point is that dependencies should preferably be pinned when deploying to production. |
We are also still elaborating our deployment procedure and we are tagging and building a package for each release. However we do a install with poetry from the tag corresponding to the release. One of the reasons is that poetry has a lock file, and that at the moment we still find it the most practical way to perform local fixes quickly. |
@fabcor-maxiv What I would find easier to understand would be having a conda-environment.yml or a requirements.txt that describes the dependencies. When you install you use those. If you want to make a change, you modify them and reinstall. It is transparent what you have and what you get. I keep getting into the situation where I make some modification (mostly commenting out the mxcubecore dependency, since I want to use the repository I have, not something poetry downloads from somewhere) , and then having to do 'poetry lock --no-update' to be able to continue.. When you are doing development it seems to me both opaque and artificial to use the output of some environment-handling program, rather than simply a list of requirements that you can edit. As for pre-commit hooks, I do not use them, except for maybe to pre-run flaks and black. As I understand it, poetry is a tool that enables you to set up in production exactly the code and configuration that was used in a particular release. The thing is, I am doing development, not production, and I work off the branch that is currently checked out in my repositories, not off releases. WRT your latest comment, you seem to be talking about setting up a production-only version, something you download as-is, put on a beamline, and run. Which is fine for those who run experiments, but I need something that allows me to do development and mock mode tests, but still allows me to update dependencies in a simple manner to the most recent changes. So I will not interfere with the best way to make production releases, but I am asking for help in getting the things that I need, which are different. I do not know how you think I ought to be working. It is possible that there is a much smarter way of doing things, if only I had learned how. I would certainly look at good advice, but clearly it is easier for me to keep working in the way I already understand. than to try to figure out an alternative to fit in with poetry. Short version: If we can get reliable and up-to-date requirements.txt files as checked in for mxcubeweb, mxcubecore, and mxcubeqt, I would love to use them. |
We drifted quite a bit from the original topic. I hope it is okay that I close this ticket. @marcus-oscarsson I realize I probably misinterpreted your comment that I was answering to in my previous message. @rhfogh Feel free to open a dedicated discussion, ping me and I will make sure to follow-up. The short of it is that I believe every team (or even every contributor) has specific needs and constraints and it is very unlikely that we will ever manage to find and maintain a "development environment story" that fits all of them. We now have one way that is documented, it has its flaws (as demonstrated by mxcube/mxcubecore#1061 for example) but maybe there is room for improvement in a manner that is not too specific to one particular team or contributor. |
Ok, to close this :) |
The CI is failing on poetry-export, it seems that we now need to install this plugin explicitly ?
Warning: poetry-plugin-export will not be installed by default in a future version of Poetry.
The text was updated successfully, but these errors were encountered: