Skip to content
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

Optimized workflow invocation step update. #2973

Merged
merged 1 commit into from Sep 29, 2016

Conversation

jmchilton
Copy link
Member

No description provided.

@nsoranzo
Copy link
Member

I think I fixed the problem in a simpler way, with the following patch my handler seems to work again:

diff --git a/lib/galaxy/model/mapping.py b/lib/galaxy/model/mapping.py
index 1fec979..8054435 100644
--- a/lib/galaxy/model/mapping.py
+++ b/lib/galaxy/model/mapping.py
@@ -2301,8 +2301,7 @@ mapper( model.WorkflowInvocation, model.WorkflowInvocation.table, properties=dic
         uselist=True,
     ),
     steps=relation( model.WorkflowInvocationStep,
-        backref='workflow_invocation',
-        lazy=False ),
+        backref='workflow_invocation' ),
     workflow=relation( model.Workflow )
 ) )

I am now running tests to check this does not break anything else. Looks good to you @jmchilton?

@jmchilton
Copy link
Member Author

jmchilton commented Sep 27, 2016

@nsoranzo There are many times we want to not lazily grab that list - I guess it might be better to go in and figure out when those are and just be explicit in those. Let me think about it and let me know how the tests work out - I wouldn't expect them to fail - the downside would be like slower workflow scheduling iterations for instance.

Update: Looking over there error - this looks fixable - it isn't a logic error or anything - just a type problem under Postgres. This would still be more optimized than just making those steps lazy load since it wouldn't require a database fetch at all.

@nsoranzo
Copy link
Member

nsoranzo commented Sep 27, 2016

@jmchilton All tests passed for my patch. Opening a PR now.

Edit: I think the non-lazy loading will hit us in other places otherwise.

@jmchilton jmchilton changed the title [WIP] Optimized workflow invocation step update. Optimized workflow invocation step update. Sep 27, 2016
@nsoranzo
Copy link
Member

Alternative solution in #2978 .

@martenson martenson merged commit 7dadb93 into galaxyproject:dev Sep 29, 2016
@jdavcs jdavcs mentioned this pull request Nov 22, 2021
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants