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

Print last line of django db connection error while waiting for db to start #565

Merged
merged 1 commit into from
Sep 17, 2021

Conversation

mk-fg
Copy link
Contributor

@mk-fg mk-fg commented Sep 2, 2021

This is to address lack of output there since 4d320f3, which can hide potentially unrelated errors.

Related Issue: #562 - see more complete description of the problem there.

New Behavior

If there are any issues connecting to database or with any kind of loaded django settings, scripts or migrations, something like this will be printed:

django.db.migrations.exceptions.NodeNotFoundError: Migration dcim.0131_consoleport_speed depends on nonexistent node ('dcim', '0130_sitegroup'). Django tried to replace migration dcim.0130_sitegroup with any of [dcim.0003_squashed_0130] but wasn't able to because some of the replaced migrations are already applied.
[ Use DB_WAIT_DEBUG=1 in netbox.env to print full traceback for errors here ]
⏳ Waiting on DB... (0s / 30s)

Instead of just the last line and no information on the error, as it was since commit 4d320f3.

I've changed var name to DB_WAIT_DEBUG to be more in line with other tunables in this part of entrypoint script.

Discussion: Benefits and Drawbacks

Pros:
Not too verbose output, clearly indicating why db connection fails or any other potential issue, suggesting a way to get more information on the error to report it or debug further.

Cons:

  • Slightly more verbose output than nothing if database is slow to start for some reason.

  • Implementation is somewhat cryptic due to need to get error code out from the process substitution.

    It doesn't need any kind of temporary files though, as don't think it's worth adding tmpfs just for this.
    Possible to do same thing in different ways, but this one seem to be pure-bash, most efficient and straightforward, aside from exec/wait wrapping around "read".

Wiki / Release Note Entry

Just a small tweak, don't think it's worth mentioning anywhere.

Double Check

  • I have read the comments and followed the PR template.
  • I have explained my PR according to the information in the comments.
  • My PR targets the develop branch.

@mk-fg
Copy link
Contributor Author

mk-fg commented Sep 2, 2021

straightforward, aside from exec/wait wrapping

Or more like "straightforward, aside from the ugly bits" :)

Let me know if something like ./manage.py showmigrations >log && break and tail -1 log might be preferrable here, or maybe just patch it over in a follow-up commit as necessary.

@tobiasge tobiasge added this to the Version 1.4.0 milestone Sep 3, 2021
@tobiasge tobiasge merged commit d432a84 into netbox-community:develop Sep 17, 2021
@tobiasge tobiasge mentioned this pull request Sep 17, 2021
3 tasks
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.

2 participants