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

Workflow query fix #60

Merged
merged 2 commits into from Feb 5, 2020
Merged

Conversation

roksys
Copy link
Contributor

@roksys roksys commented Feb 5, 2020

No description provided.

Rokas Maciulaitis added 2 commits February 5, 2020 15:48
* psql allows to pass run number as a string while sqlite not
@@ -103,7 +103,7 @@ def _get_workflow_with_uuid_or_name(uuid_or_name, user_uuid):
# `run_number` was specified.
# Check `run_number` is valid.
try:
float(run_number)
run_number = float(run_number)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, looking at instances of float and int casting in the code base, and looking at possible RDBMS differences in handling floats, what about:

  • either forcing the run number to be float everywhere on the cluster side (e.g. 42.0) and hiding the 42.0 complexity into 42 on the reana-client side?

  • or using two integer columns, run_number and rerun_number, in the DB model, each properly indexed for speed, and defining proper getter/setter hybrid property for run_number lookups that would look in both columns based on run_number string without any needs for float casting?

It might be more robust that way, i.e. considering run number as a string composed of two integers separated by dot, and not casting anything. Let's muse IRL?

@tiborsimko tiborsimko merged commit e48ac8f into reanahub:master Feb 5, 2020
@tiborsimko tiborsimko added this to Done in Awesome-Workshop-Basics via automation Feb 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants