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

fix issue#3160 #3161

Merged
merged 1 commit into from
Jun 22, 2021
Merged

Conversation

sameh-farouk
Copy link
Member

@sameh-farouk sameh-farouk commented Jun 5, 2021

Description

adding extra checks to make sure of the real state of Nginx when threebot server starts.

Changes

is_running() now will check if the PID file specified in the Nginx configuration file Is exists, contains PID that belongs to a running Nginx master process.

Related Issues

#3160

Checklist

  • Pre-commit hook is installed to do formatting checks before committing code...etc
  • Tests included
  • Build pass
  • Documentation
  • Code format and docstrings

@sameh-farouk sameh-farouk marked this pull request as draft June 5, 2021 20:07
@sameh-farouk sameh-farouk force-pushed the development-tools-nginx-start-fix-iss3160 branch from 4d482a4 to 11a1c1e Compare June 6, 2021 11:43
@sameh-farouk sameh-farouk marked this pull request as ready for review June 6, 2021 11:50
@sameh-farouk sameh-farouk force-pushed the development-tools-nginx-start-fix-iss3160 branch from 39f16b9 to 873e0fc Compare June 6, 2021 12:33
@abom
Copy link
Contributor

abom commented Jun 6, 2021

What's the behavior if we removed the check cmd? I think it would do this check and see if the process is running, can we test this?

@sameh-farouk
Copy link
Member Author

sameh-farouk commented Jun 6, 2021

What's the behavior if we removed the check cmd? I think it would do this check and see if the process is running, can we test this?

@abom but doing so will return us to this #3116 ?

@xmonader xmonader merged commit 2149223 into development Jun 22, 2021
@xmonader xmonader deleted the development-tools-nginx-start-fix-iss3160 branch June 22, 2021 08:42
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.

4 participants