Skip to content

Add columns during edit #1016

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

Closed
wants to merge 4 commits into from

Conversation

squirrelo
Copy link
Contributor

Since this is needed for EBI submission, this should get reviewed and merged quickly.

This allows addition of columns during editing of a sample template. It also cleans up the add and remove category functions, as they were missing the addition of the column names to the study_sample_columns table.

@josenavas
Copy link
Contributor

@squirrelo besides the problems connecting to the redis server, it looks like there is a legitimate test failure!

@squirrelo
Copy link
Contributor Author

Seems like only redis failures now.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.06%) to 79.15% when pulling 56a25bc on squirrelo:add-cols-using-edit into 54f63e9 on biocore:master.

@squirrelo
Copy link
Contributor Author

ready for review

if default is not None:
# mogrify the sql and add to end
with conn_handler.get_postgres_cursor() as cur:
sql = sql + cur.mogrify(" NOT NULL DEFAULT %s", [default])
Copy link
Contributor

Choose a reason for hiding this comment

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

Check the comment of the first answer on this thread. In short, mogrify should be used only for debugging, not for generating code. Anyways, I also find this solution more convoluted, so if you can just add the if and generate either of the sql in the specific case of the if would be more cleaner/maintainable.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.06%) to 79.16% when pulling 28ae032 on squirrelo:add-cols-using-edit into 54f63e9 on biocore:master.

@josenavas
Copy link
Contributor

Closing in favor of #1021

@josenavas josenavas closed this Mar 30, 2015
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.

3 participants