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

Proper Gunicorn bringup detection #2842

Merged
merged 2 commits into from
Jul 2, 2024
Merged

Proper Gunicorn bringup detection #2842

merged 2 commits into from
Jul 2, 2024

Conversation

Ulincsys
Copy link
Contributor

@Ulincsys Ulincsys commented Jul 1, 2024

Description

  • Add /api route to get the location of the backend API
  • Check that Gunicorn has actually started during backend start invocation
    • Poll the process state every 0.5 seconds
    • Make GET request to /api to check that service has started
    • Repeat until the state of the service can be determined

Future Work

  • Implement timeout in case Gunicorn takes too long to respond or gets stuck during startup
    • Currently this waits until either the process dies or the server responds

Signed commits

  • Yes, I signed my commits.

logger.info('Gunicorn webserver started...')
logger.info(f'Augur is running at: {"http" if development else "https"}://{host}:{port}')
logger.info(f"The API is available at '{api_response.json()['route']}'")

processes = start_celery_worker_processes(float(worker_vmem_cap), disable_collection)
Copy link

Choose a reason for hiding this comment

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

[pylint] reported by reviewdog 🐶
W0621: Redefining name 'processes' from outer scope (line 383) (redefined-outer-name)

augur/application/cli/backend.py Show resolved Hide resolved
augur/application/cli/backend.py Show resolved Hide resolved
augur/application/cli/backend.py Show resolved Hide resolved
augur/application/cli/backend.py Show resolved Hide resolved
@@ -91,7 +108,6 @@ def start(ctx, disable_collection, development, port):
celery_beat_process = subprocess.Popen(celery_command.split(" "))
Copy link

Choose a reason for hiding this comment

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

[pylint] reported by reviewdog 🐶
R1732: Consider using 'with' for resource-allocating operations (consider-using-with)

augur/application/cli/backend.py Show resolved Hide resolved
logger.info('Gunicorn webserver started...')
logger.info(f'Augur is running at: {"http" if development else "https"}://{host}:{port}')
logger.info(f"The API is available at '{api_response.json()['route']}'")

processes = start_celery_worker_processes(float(worker_vmem_cap), disable_collection)
Copy link

Choose a reason for hiding this comment

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

[pylint] reported by reviewdog 🐶
W0621: Redefining name 'processes' from outer scope (line 386) (redefined-outer-name)

augur/application/cli/backend.py Show resolved Hide resolved
augur/application/cli/backend.py Show resolved Hide resolved
augur/application/cli/backend.py Show resolved Hide resolved
augur/application/cli/backend.py Show resolved Hide resolved
Copy link
Member

@sgoggins sgoggins left a comment

Choose a reason for hiding this comment

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

Looks worth testing after this coming release since we are already testing.

@sgoggins sgoggins merged commit c43ca39 into dev Jul 2, 2024
9 checks passed
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