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

Clean db #670

Merged
merged 9 commits into from
May 6, 2017
Merged

Clean db #670

merged 9 commits into from
May 6, 2017

Conversation

sjanssen2
Copy link
Contributor

Remove unused tables, see issue #669

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 64.396% when pulling f17d6b7 on sjanssen2:cleanDB into 8c8023b on biocore:master.

@josenavas
Copy link
Member

@sjanssen2 please read the "Making Database Changes" section of Qiita's contributing file. That applies to all our projects that involve DB but it is on the Qiita as we developed those docs for that project and we didn't want to duplicate the contents of that file on multiple repos.

Basically, you need to update the dbschema files (dbs and html). That is how we avoid them going out of sync, and that's why I pointed out on my email that the first task to perform is sync the dbschema files with the DB. The more they go out of sync the harder will be to sync them again.


-- a table we only read from, but never write. Thus, it is static. Can't we correct information elsewhere and get rid of this table?
-- see discussion: https://github.com/biocore/american-gut-web/issues/669
--DROP TABLE ag.duplicate_consents;
Copy link
Member

Choose a reason for hiding this comment

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

Remove commented code

Copy link
Member

Choose a reason for hiding this comment

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

👍 and or remove that table?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am back to power now.
I think that this table will contradict with what we are going to implement with the many-to-many relation "source". And since @josenavas thinks that those information have never been validated, I'd like to remove this table. However, @EmbrietteH and @wasade needs to confirm this please.

@wasade
Copy link
Member

wasade commented May 5, 2017

Seems good barring @josenavas's comments

@sjanssen2
Copy link
Contributor Author

I am running out of battery power :-(

@wasade
Copy link
Member

wasade commented May 5, 2017 via email

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 64.396% when pulling b1274cc on sjanssen2:cleanDB into 8c8023b on biocore:master.

@sjanssen2
Copy link
Contributor Author

I cannot find version 6.1.0 of DbSchema as a downloadable package, thus I am not able to create the HTML. Can someone else step in or point me to a download location please.

@sjanssen2
Copy link
Contributor Author

I manually compared ag.dbs and actual table columns (with pgAdmin) and modified our schema where it was not in sync with the DB. Should be fine now.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 64.396% when pulling 1098860 on sjanssen2:cleanDB into 8c8023b on biocore:master.

@wasade wasade merged commit 9fe5f95 into biocore:master May 6, 2017
@sjanssen2
Copy link
Contributor Author

Hi @wasade . Thanks for the late merge!
Do you know what was wrong with our duct tape solution, other than it was duct tape? Did it return correct results? Looks like I wasn't in the full loop of critique about this piece of code.

@wasade
Copy link
Member

wasade commented May 6, 2017

I never saw results it produced from the live db. @EmbrietteH, did you get a chance to review records from the "ducttape" for pulling metadata from participants with multiple distinct surveys (eg main, fermented food, etc)?

I'm not sure what you mean by wrong with the solution? It was a hack to mimic the many-many relationships iirc

@sjanssen2
Copy link
Contributor Author

I want to double check that the SQL code you created for this solution returns correct data.
Before I left to Germany, I sent an example pulldown to @EmbrietteH but never got a response.

@EmbrietteH
Copy link
Contributor

EmbrietteH commented May 6, 2017 via email

@sjanssen2
Copy link
Contributor Author

Sorry, I was looking in my mail box for statements why my pulldown didn't work. You did reply me - my fault.

@EmbrietteH
Copy link
Contributor

EmbrietteH commented May 6, 2017 via email

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.

5 participants