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

rework database creation, cleanup + other db fixes #1981

Merged
merged 6 commits into from
Feb 10, 2016

Conversation

rutsky
Copy link
Member

@rutsky rutsky commented Feb 9, 2016

No description provided.

@rutsky rutsky changed the title rework database creation, cleanup + other db fixes [WIP] rework database creation, cleanup + other db fixes Feb 9, 2016
When tables are created using create_all() not only tables created,
but also SQLAlchemy runs its hacks, e.g. for cyclic referencew with
use_alter=True.

With this change some tables sets are not created in databases that
enforces relations between tables (e.g. PostgreSQL), because that set
of tables contained references on not created tables.
@codecov-io
Copy link

Current coverage is 84.25%

Merging #1981 into master will not affect coverage as of 7e1a021

@@            master   #1981   diff @@
======================================
  Files          320     320       
  Stmts        31172   31172       
  Branches         0       0       
  Methods          0       0       
======================================
  Hit          26265   26265       
  Partial          0       0       
  Missed        4907    4907       

Review entire Coverage Diff as of 7e1a021


Uncovered Suggestions

  1. +0.11% via ...ot/status/builder.py#381...414
  2. +0.10% via ...ot/status/builder.py#453...481
  3. +0.09% via ...dbot/changes/mail.py#478...505
  4. See 7 more...

Powered by Codecov. Updated on successful CI builds.

@rutsky
Copy link
Member Author

rutsky commented Feb 10, 2016

I merged Travis config from PR #1982 to this PR.
PR #1982 shows, that there are errors when file-based SQLite database is used, and after merging of PR #1982 commits we should see, that there is no more errors (or at least less number of errors :) ) --- so this PR is actually fixing something.

@@ -14,6 +14,7 @@ env:
- TWISTED=13.0.0 SQLALCHEMY=latest TESTS=trial
- TWISTED=15.4.0 SQLALCHEMY=latest TESTS=trial
- TWISTED=latest SQLALCHEMY=latest TESTS=trial
- TWISTED=latest SQLALCHEMY=latest TESTS=trial BUILDBOT_TEST_DB_URL=sqlite:////tmp/test_db.sqlite
Copy link
Member

Choose a reason for hiding this comment

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

Can you include a comment about why this is useful, compared to the default memory DB?

@djmitche
Copy link
Member

This looks good aside from the stylistic comment.

@rutsky
Copy link
Member Author

rutsky commented Feb 10, 2016

@djmitche, I fixed issues that you noted.

@rutsky
Copy link
Member Author

rutsky commented Feb 10, 2016

@djmitche, is my fixes looks good to you?

@djmitche
Copy link
Member

@rutsky perfect, yes

rutsky added a commit that referenced this pull request Feb 10, 2016
rework database creation, cleanup + other db fixes
@rutsky rutsky merged commit 99f9798 into buildbot:master Feb 10, 2016
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