Add Conda Lock Updating Action #5

ajelinski · 2020-12-30T23:14:34Z

This GitHub Action creates or updates Conda Lock from the freshly created Conda Environment based on the Conda environment.yml file. The Pull Request is automatically issued if there is any change in the Conda Lock.

The workflow using this Action is available at: https://github.com/antmicro/symbiflow-examples-bot/blob/master/.github/workflows/update_conda_locks.yml
The example workflow jobs can be checked at: https://github.com/antmicro/symbiflow-examples-bot/actions/runs/453502941
The lock is quite fresh in there on the master branch so apart from the environment's name change (due to the script change) in the latest PR there's only one package updated but it can be seen how the Conda Lock looks like: https://github.com/antmicro/symbiflow-examples-bot/pull/41/files

ajelinski · 2020-12-30T23:32:37Z

I can see that I need to add #.*coding: utf-8 and LICENSE to satisfy the Checks test. Should the license simply resemble that found in the other files @mithro @umarcor? https://github.com/SymbiFlow/actions/blob/77e0cafc07a9dfbe39d41aad30c57c0211ba5af6/checks/check_license.sh#L3-L7

update_conda_lock/action.yml

umarcor · 2020-12-30T23:35:55Z

I can see that I need to add #.*coding: utf-8 and LICENSE to satisfy the Checks test. Should the license simply resemble that found in the other files @mithro @umarcor?

Yes, please. Unless you have any specific requirement.

umarcor · 2020-12-30T23:38:22Z

Please, add it to the README too.

ajelinski · 2020-12-31T02:04:27Z

I had to make some small fixes after testing on symbiflow-arch-defs CI. I've fixed failing checks although I have to make sure about the copyright. Therefore I'll make it draft for now.

mithro · 2021-04-19T14:20:47Z

@ajelinski - Ping? I would like to get this GitHub action moving again?

ajelinski · 2021-04-29T15:24:43Z

I still need to test if everything works well after these changes but please take a look if you like it more now @umarcor.

ajelinski · 2021-04-29T23:39:18Z

Rebased and fixed after testing on a symbiflow-examples repository clone. A PR created by this action is e.g.
antmicro/symbiflow-examples-bot#142 (created during https://github.com/antmicro/symbiflow-examples-bot/actions/runs/797726587).

GitHub
antmicro/symbiflow-examples-bot
Contribute to antmicro/symbiflow-examples-bot development by creating an account on GitHub.

umarcor

The structure looks much nicer and easier to understand/maintain. Congrats for the cleanup!

ajelinski · 2021-05-13T18:02:32Z

I'm glad you like it @umarcor!

I've cleaned the commit history. I think now this PR is ready to be merged.

ajelinski · 2021-05-21T08:38:56Z

I tested the bot again to make sure that everything still works as intended before it's merged.

Somehow after separating scripts and steps I skipped the test of the special case when the lock is up to date. It needed a little modification to work properly (exit 0 was enough when it was a single script): https://github.com/SymbiFlow/actions/compare/d358d22afdaf54376197be3bd1c9af265e79ecb2..6970a7e6257a3e66dec67b97cd51c44a0c34ad5b

BTW there's also an issue with pip that doesn't handle simultaneous installation of git-based packages with their git-based dependencies properly. I've found a way to avoid it, i.e. making pip ignore package dependencies when Conda creates the environment based on the Conda Lock. It is absolutely safe to do since all packages, including the dependencies, are in the Conda Lock anyway.

I have created an issue on pip so maybe it won't be needed in the future: pypa/pip#10002

For now, I added information on how to handle such a situation to the README: eec0e64 .

Could you please take another look at the action changes @umarcor? Now the action is definitely ready to be merged if you find them acceptable.

GitHub
SymbiFlow/actions
GitHub Actions to be reused in CI workflows. Contribute to SymbiFlow/actions development by creating an account on GitHub.

umarcor · 2021-05-21T12:12:33Z

LGTM! @mithro, I'll let you merge 😉

mithro · 2021-05-21T12:41:11Z

BTW there's also an issue with pip that doesn't handle simultaneous installation of git-based packages with their git-based dependencies properly. I've found a way to avoid it, i.e. making pip ignore package dependencies when Conda creates the environment based on the Conda Lock. It is absolutely safe to do since all packages, including the dependencies, are in the Conda Lock anyway.

I have created an issue on pip so maybe it won't be needed in the future: pypa/pip#10002

We should not have any packages which use the @ symbol in their install_requires as we determined that functionality is fundamentally broken. Instead the install_requires should list the package name and then the requirements.txt should specify how/where to get the package from (IE a git+http:// URL).

mithro

Some minor formatting / style issues.

Can you run pylint or flake8 on your Python files?

README.md

mithro · 2021-05-21T12:46:52Z

update_conda_lock/action.yml

+    run:   $GITHUB_ACTION_PATH/check_prepare_env.sh
+
+  - shell: bash
+    run: |


This would probably be better as a Python script?

I'm not sure if you meant it this way but I can't make it a separate script since it uses GH Action inputs so it has to be in the action.yml file.

If you thought about only converting it to Python but leaving it in action.yml then I'm not sure. Why would it be better?

mithro · 2021-05-21T12:48:03Z

update_conda_lock/check_prepare_env.sh

@@ -0,0 +1,48 @@
+#!/bin/bash


If we make this file Python it can work on windows / mac / linux? We already require Python for other parts.

This script should work on all these OSs, there's nothing really OS-specific.

mithro · 2021-05-21T12:49:15Z

update_conda_lock/push_updated_lock.sh

@@ -0,0 +1,50 @@
+#!/bin/bash


If we make this file Python it can work on windows / mac / linux? We already require Python for other parts?

The script itself should work on all these OSs but I'm not sure about update_lock.py on Windows with its \r\n newlines as there are some file manipulations involved.

update_conda_lock/create_pull_request.py

update_conda_lock/update_lock.py

Signed-off-by: Adam Jeliński <[email protected]>

ajelinski · 2021-05-27T17:27:21Z

BTW there's also an issue with pip that doesn't handle simultaneous installation of git-based packages with their git-based dependencies properly. I've found a way to avoid it, i.e. making pip ignore package dependencies when Conda creates the environment based on the Conda Lock. It is absolutely safe to do since all packages, including the dependencies, are in the Conda Lock anyway.
I have created an issue on pip so maybe it won't be needed in the future: pypa/pip#10002

We should not have any packages which use the @ symbol in their install_requires as we determined that functionality is fundamentally broken. Instead the install_requires should list the package name and then the requirements.txt should specify how/where to get the package from (IE a git+http:// URL).

From what I saw working with pip on this action I can only agree but can you show me some example of this cooperation between install_requires and the requirements.txt?

ajelinski · 2021-05-27T17:51:41Z

The Python scripts had many changes, e.g. docstrings were added, since I found many parts not easily understandable after these few months. They fully conform to the flake8 and pylint rules now.

ajelinski · 2021-08-18T09:40:12Z

@mithro Should I make any changes to have this PR merged?

mithro · 2021-08-18T23:55:02Z

Check with @acomodi and @kgugala - If they are happy merge away!

acomodi · 2021-08-19T07:42:59Z

@ajelinski LGTM

mithro · 2021-08-19T14:52:52Z

@acomodi - Now we just need to deploy this everywhere?

acomodi · 2021-08-19T15:06:57Z

@mithro Yes, we can test this right away with arch-defs for starters

mithro · 2021-08-19T15:08:51Z

Would symbiflow-examples be an easier starting repo?

ajelinski force-pushed the add_conda_locker branch from cb04f99 to a742ec5 Compare December 30, 2020 23:16

umarcor reviewed Dec 30, 2020

View reviewed changes

ajelinski force-pushed the add_conda_locker branch 2 times, most recently from da38492 to 4c154b8 Compare December 31, 2020 01:49

ajelinski marked this pull request as draft December 31, 2020 02:04

ajelinski mentioned this pull request Dec 31, 2020

Add bot locking and updating Conda and Pip packages f4pga/f4pga-arch-defs#1885

Merged

ajelinski force-pushed the add_conda_locker branch 3 times, most recently from e2cf5d3 to 0d69a68 Compare April 29, 2021 23:17

umarcor approved these changes May 7, 2021

View reviewed changes

ajelinski force-pushed the add_conda_locker branch from 92f43a4 to d358d22 Compare May 13, 2021 16:38

ajelinski marked this pull request as ready for review May 13, 2021 16:39

ajelinski force-pushed the add_conda_locker branch 2 times, most recently from c7ccc09 to eec0e64 Compare May 21, 2021 08:15

mithro approved these changes May 21, 2021

View reviewed changes

Add update_conda_lock action

d2dbe3c

Signed-off-by: Adam Jeliński <[email protected]>

ajelinski force-pushed the add_conda_locker branch from eec0e64 to b846ece Compare May 27, 2021 17:12

README: Add update_conda_lock information

fb771da

Signed-off-by: Adam Jeliński <[email protected]>

ajelinski force-pushed the add_conda_locker branch from b846ece to fb771da Compare May 27, 2021 17:17

kgugala merged commit e22f88d into f4pga:main Aug 19, 2021

ajelinski deleted the add_conda_locker branch August 19, 2021 08:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Conda Lock Updating Action #5

Add Conda Lock Updating Action #5

ajelinski commented Dec 30, 2020

ajelinski commented Dec 30, 2020

umarcor commented Dec 30, 2020

umarcor commented Dec 30, 2020

ajelinski commented Dec 31, 2020

mithro commented Apr 19, 2021

ajelinski commented Apr 29, 2021

ajelinski commented Apr 29, 2021 •

edited by unfurl-links bot

Loading

umarcor left a comment

ajelinski commented May 13, 2021

ajelinski commented May 21, 2021 •

edited by unfurl-links bot

Loading

umarcor commented May 21, 2021

mithro commented May 21, 2021

mithro left a comment

mithro May 21, 2021

ajelinski May 27, 2021 •

edited

Loading

mithro May 21, 2021

ajelinski May 27, 2021

mithro May 21, 2021

ajelinski May 27, 2021

ajelinski commented May 27, 2021

ajelinski commented May 27, 2021 •

edited

Loading

ajelinski commented Aug 18, 2021

mithro commented Aug 18, 2021

acomodi commented Aug 19, 2021

mithro commented Aug 19, 2021

acomodi commented Aug 19, 2021

mithro commented Aug 19, 2021

Add Conda Lock Updating Action #5

Add Conda Lock Updating Action #5

Conversation

ajelinski commented Dec 30, 2020

ajelinski commented Dec 30, 2020

umarcor commented Dec 30, 2020

umarcor commented Dec 30, 2020

ajelinski commented Dec 31, 2020

mithro commented Apr 19, 2021

ajelinski commented Apr 29, 2021

ajelinski commented Apr 29, 2021 • edited by unfurl-links bot Loading

umarcor left a comment

Choose a reason for hiding this comment

ajelinski commented May 13, 2021

ajelinski commented May 21, 2021 • edited by unfurl-links bot Loading

umarcor commented May 21, 2021

mithro commented May 21, 2021

mithro left a comment

Choose a reason for hiding this comment

mithro May 21, 2021

Choose a reason for hiding this comment

ajelinski May 27, 2021 • edited Loading

Choose a reason for hiding this comment

mithro May 21, 2021

Choose a reason for hiding this comment

ajelinski May 27, 2021

Choose a reason for hiding this comment

mithro May 21, 2021

Choose a reason for hiding this comment

ajelinski May 27, 2021

Choose a reason for hiding this comment

ajelinski commented May 27, 2021

ajelinski commented May 27, 2021 • edited Loading

ajelinski commented Aug 18, 2021

mithro commented Aug 18, 2021

acomodi commented Aug 19, 2021

mithro commented Aug 19, 2021

acomodi commented Aug 19, 2021

mithro commented Aug 19, 2021

ajelinski commented Apr 29, 2021 •

edited by unfurl-links bot

Loading

ajelinski commented May 21, 2021 •

edited by unfurl-links bot

Loading

ajelinski May 27, 2021 •

edited

Loading

ajelinski commented May 27, 2021 •

edited

Loading