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

Commands datatype table #183

Merged
merged 8 commits into from
Jul 2, 2014

Conversation

squirrelo
Copy link
Contributor

This adds a join table that links a command up to the datatypes it can be run on. Needed so we can programmatically populate the commands selection for each datatype.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 94812b3 on squirrelo:commands-datatype-table into 59e835c on biocore:master.

@@ -107,7 +107,7 @@ def _sql_executor(self, sql, sql_args=None, many=False):
err_sql = cur.mogrify(sql, sql_args)
else:
err_sql = cur.mogrify(sql, sql_args[0])
except IndexError:
except (IndexError, TypeError):
Copy link
Contributor

Choose a reason for hiding this comment

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

In what context would a TypeError be raised above?

Copy link
Contributor

Choose a reason for hiding this comment

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

in case that sql_args is None

Copy link
Contributor

Choose a reason for hiding this comment

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

In that case, i think this try/except can be replaced with:

if sql_args and sql_args[0] and isinstance(sql_args[0], Iterable):
    err_sql = cur.mogrify(sql, sql_args[0])
else:
    err_sql = cur.mogrify(sql, sql_args)

right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Uhm... That should work.

Copy link
Contributor

Choose a reason for hiding this comment

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

you could even remove the and sql_args[0] part of the if clause, i think

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, fix pushed.

@antgonza
Copy link
Member

antgonza commented Jul 2, 2014

Looks good to me, anyone else so we can merge?

@adamrp
Copy link
Contributor

adamrp commented Jul 2, 2014

After that comment about the nested try/excepts is addressed, I give it the 👍

@coveralls
Copy link

Coverage Status

Coverage increased (+0.07%) when pulling 10d09d6 on squirrelo:commands-datatype-table into 59e835c on biocore:master.

@squirrelo
Copy link
Contributor Author

This is ready to merge.

adamrp added a commit that referenced this pull request Jul 2, 2014
@adamrp adamrp merged commit 0c6711f into qiita-spots:master Jul 2, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants