mvidalgarcia
commented
2 years ago @matthewfeickert @lukasheinrich maybe you have some ideas on what's going on here?
Closed mvidalgarcia closed 2 years ago
mvidalgarcia
commented
2 years ago @matthewfeickert @lukasheinrich maybe you have some ideas on what's going on here?
matthewfeickert
commented
2 years ago With
adage==0.10.1(andnetworkx==1.11) everything runs as usual.
From the job logs it looks like you're running python:2.7-slim. We don't support Python 2, and haven't for a while, but it was only in adage v0.10.2 that this got properly reflected with python_requires (to keep from breaking Python 2 installs).
After upgrading to
adage==0.10.2, it's required to upgradenetworkx>=2.4
The reason for setting lower bounds on networkx is to allow for Python 3.9+ support. c.f.
for more details.
Though if you're running Python 2, I'm not sure how you ever installed adage==0.10.2. :? Is it because in https://github.com/mvidalgarcia/reana-workflow-engine-yadage/commit/b9e1eecf87ad4d357cb7e716a73b3fc9144a81cb you're using pip-tools to compile your dependencies outside of your runtime and so creating situations in which pip-tools can resolve things in a Python 3 environment but your Python 2 runtime can't?
matthewfeickert
commented
2 years ago It also looks like your install_requires has additional situations in which you'd create conflicts from pinning dependencies
so I'm not sure how pip-tools is resolving those in anyway that wouldn't give a breaking build if you installed adage==0.10.2.
mvidalgarcia
commented
2 years ago I'm not running Python 2, that's just the container where the job is being run. The Yadage workflow engine uses Python 3.8.
matthewfeickert
commented
2 years ago I'm not running Python 2, that's just the container where the job is being run. The Yadage workflow engine uses Python 3.8.
:+1: and I see that you're using pip-tools (also :+1:)
to produce a requirements lockfile that has
https://github.com/reanahub/reana-workflow-engine-yadage/blob/ac194f6d2972ea77642ba5d3f10d2da2173fff0e/requirements.txt#L7 https://github.com/reanahub/reana-workflow-engine-yadage/blob/ac194f6d2972ea77642ba5d3f10d2da2173fff0e/requirements.txt#L32
So unless you're also updating your setup.py to avoid having pins like
there's never going to be a way for you to have pip-tools properly resolve things where you're giving pip-tools a scenario with adage>=0.10.2.
Am I missing something here about what you're doing? Or are you saying that you're already removing the pins and then letting pip-tools do the resolution and then you're getting the bug?
mvidalgarcia
commented
2 years ago Or are you saying that you're already removing the pins and then letting
pip-toolsdo the resolution and then you're getting the bug?
Exactly this. Sorry for the misunderstanding.
matthewfeickert
commented
2 years ago Exactly this. Sorry for the misunderstanding.
Okay cool. :+1: Can you give me a branch that has a minimal failing example? I know that you've already explained that apparently removing the networkx requirement is causing this, but it would be helpful to see exactly what is being done.
matthewfeickert
commented
2 years ago Couldn't import dot_parser, loading of dot files will not be possible.
(Thanks to @lukasheinrich for pointing out that I missed this.)
@mvidalgarcia Sorry can you also try installing pydot? pydot2 hasn't released since 2014 and doesn't support Python 3.9+ (c.f. https://github.com/yadage/adage/pull/17) so I'm now treating it as abandoned/deprecated.
My guess is from
this is because of us (or at least the yadage[viz] extra being a bit broken at the moment so everyone has been installing its extra dependencies by hand). Can you try again with adding pydot in and then if that works try removing pydot2 as well?
edit: I'm trying with
$ git diff origin/master -- setup.py
diff --git a/setup.py b/setup.py
index 0fce759..c6b6b3a 100644
--- a/setup.py
+++ b/setup.py
@@ -44,9 +44,7 @@ install_requires = [
"checksumdir>=1.1.4,<1.2",
"graphviz>=0.12", # FIXME needed only if yadage visuale=True.
"mock>=3.0,<4",
- "networkx==1.11",
- "packtivity==0.14.21",
- "pydot2>=1.0.33", # FIXME needed only if yadage visuale=True.
+ "pydot>=1.0.29", # FIXME needed only if yadage visuale=True.
"pydotplus>=2.0.2", # FIXME needed only if yadage visuale=True.
"pygraphviz>=1.5", # FIXME needed only if yadage visuale=True.
"reana-commons[yadage]>=0.8.0,<0.9.0",at the moment, and then updating the requirements.txt with pip-tools
$ git diff origin/master -- requirements.txt | grep '==' | grep '+' | grep 'adage\|packtivity\|pydot'
+adage==0.10.2
+packtivity==0.14.24
+pydot==1.4.2
+pydotplus==2.0.2
+reana-commons[yadage]==0.8.2
+yadage==0.20.1
+yadage-schemas==0.10.7which I can get to build fine with
./run-tests.sh --check-docker-buildbut I'm not clear on how to run your minimal failing example.
mvidalgarcia
commented
2 years ago @matthewfeickert Your diff did the trick! Indeed it seems the error was caused by pydot2. I'm running some examples locally and they're working now, no adage pinning needed. I'll push my changes soon to this PR.
Thanks for the quick reaction!
Edit: Here are the changes: https://github.com/reanahub/reana-workflow-engine-yadage/compare/4b6e2bd8b37566ffef132291fb32ef796a3c3a7b..25d17851d37ec285c9103a15b1cf3b8c0c856623
matthewfeickert
commented
2 years ago Your diff did the trick! Indeed it seems the error was caused by
pydot2. I'm running some examples locally and they're working now, noadagepinning needed. I'll push my changes soon to this PR.
@mvidalgarcia Great! :) Yeah, sorry about all of this as this is indeed the fault of recent yadage org stuff as we've been trying to clean things up. Specifically, when going to adage v0.10.2 we switched to using the networkx v2.X API (https://github.com/yadage/adage/pull/18) and the networkx v2 API change brought with it a lot of related changes for graphviz visualization (c.f. https://github.com/networkx/networkx/pull/2272).
On the upside though, I've made an adage v0.10.3 and am working on getting a yadage v0.20.2 which should have this visualization stuff sorted out in their viz extras so you won't need to mess with any of the stuff in setup.py that currently has the # FIXME needed only if yadage visuale=True. comment (c.f. https://github.com/yadage/adage/pull/25, https://github.com/yadage/yadage/pull/108). You should be able to just do yadage[viz]. :+1: I'll still work on getting pydotplus removed though in a later release (https://github.com/yadage/yadage/issues/106).
matthewfeickert
commented
2 years ago The regression seen in https://github.com/reanahub/reana-workflow-engine-yadage/pull/219 is caused by https://github.com/yadage/yadage/issues/116.
After upgrading to
adage==0.10.2, it's required to upgradenetworkx>=2.4. This makes our yadage workflows fail on the workflow engine side. The jobs finished successfully but there's a problem afterward, perhaps when the visualization files are generated.With
adage==0.10.1(andnetworkx==1.11) everything runs as usual.Adage release: https://github.com/yadage/adage/releases/tag/v0.10.2