hackoregon / devops-17

deployment tools for Hack Oregon projects
4 stars 3 forks source link

Error: "test database already exists" - only during Travis builds of the "push" #46

Open MikeTheCanuck opened 7 years ago

MikeTheCanuck commented 7 years ago

When I push a PR to our Budget backend repo, Travis fires off two builds:

The "push" build always fails, and the "pr" build always passes.

When the "push" build fails, this is the tail end of the Travis build log:

Creating test database for alias 'default'...
Got an error creating the test database: database "test_budget" already exists
Destroying old test database for alias 'default'...
Got an error recreating the test database: database "test_budget" is being accessed by other users
DETAIL:  There is 1 other session using the database.
The command "./budget_proj/bin/test-proj.sh -t" exited with 2.
Done. Your build exited with 1.

I have already made sure that I'm running the manage.py test --no-input command - so that python is supposed to ignore any warnings like this.

I want to stop this error from coming up, because it isn't a good habit to get into, me ignoring a broken build and merging the PR despite that.

MikeTheCanuck commented 7 years ago

Thought: if I'm understanding discussions like this one, it seems that Django actually creates a database instance on the live PostgreSQL backend. [I'd assumed the so-called test database was something it was creating in some local database it was managing just for testing. I still don't know exactly why Django needs to create a database instance for testing, but I'm likely to dig into that very soon.]

If that's true, and if this error only comes up during PR builds, it's because we have two builds running pretty much simultaneously, and one build tries to wrap up testing while the other build is still using the test database.

Thus, this is likely an inevitable and unavoidable collision, the only solution to which is to find some way to prevent GitHub/Travis from firing off two builds every time we create a PR for the project.

I believe that we should always have one build to validate that the PR will result in a successful deployment, so my previous experiments that led to no builds in this scenario aren't helpful.

In the meantime, our workaround is to assume that when we have one successful build out of the two, that that means we're good to merge. [Which is what I've been optimistically assuming all along.]

MikeTheCanuck commented 7 years ago

The Django testing docs explain that in cases where you wish to preserve (i.e. not destroy) an existing test database, you can use the --keepdb option.

Not sure if that makes sense in this case, where Django is attempting to destroy a database it encounters (due to a parallel job running) - what if in the future we decide to start writing data to the database as part of the test suite? If that happens, we could easily end up creating a situation where build #1 creates a record that build #2 encounters, throwing off the expected record count [or something more serious].

I suspect the right answer is "once we start adding tests that write new data, we should instantiate a new dummy database so that we don't end up colliding". However, isn't that already what Django is doing - creating a "test database" alongside the production database? And if true, then isn't it the case that we would have to tell Django to create a new, randomly-named "test database" to avoid collisions of parallel builds (e.g. using TRAVIS_BUILD_NUMBER as the randomization bit)?

Much to think about...

bhgrant8 commented 7 years ago

Not sure of the mechanics of travis and its db environments, but one can set different db variables for different environments, ie testing vs prod.

Importing sys package into settings.py then allows you to specify a different db when running 'manage.py test' (This can be used to spin up a local testdb instead of on aws)

One can also specify the name of the test database in settings.

Maybe something like:

if 'test' in sys.argv or 'test_coverage' in sys.argv: DATABASES = { 'default': { 'ENGINE': project_config.AWS['ENGINE'], 'NAME': project_config.AWS['NAME'], 'HOST': project_config.AWS['HOST'], 'PORT': 5432, 'USER': project_config.AWS['USER'], 'PASSWORD': project_config.AWS['PASSWORD'], 'TEST': { 'NAME': 'test_fire' + python function for random number, }, }

Could solve?

MikeTheCanuck commented 7 years ago

Thanks @BrianHGrant - that sounds like a likely candidate.

I ended up researching further and I came up with an idea I'd like to pursue - I'm going to add the following to the DATABASES list in settings.py:

    'TEST': {
        'NAME': 'test_' + 'NAME' + os.getenv('TRAVIS_BUILD_NUMBER', "") # TRAVIS_BUILD_NUMBER ignored if not available, such as when performing a local build
        }

IIUC, the TEST variable should just be ignored for non-test work, so I should be able to leave that in the project_config.py without even wrapping it in a conditional like if 'test' in sys.argv or 'test.coverage' in sys.argv:. Or so I'm hoping.

MikeTheCanuck commented 7 years ago

Oh, good to know! That's probably going to be important in the future (and maybe now if my crazy idea doesn't pay off).

MikeTheCanuck commented 7 years ago

The results of one PR (#66 in Test Budget) do not hit this issue, though it appears the "push" build finished long before the "pr" build. Still, progress!

MikeTheCanuck commented 7 years ago

Weird, after pushing another commit to PR #66, Travis failed the pr build but passed the push build.

Which implies the change I added to settings.py isn't a 100% solution. Maybe there's something that has to be added to the repo code to pick up the TEST change?