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

Fixes for server command-line #1093

Merged
merged 4 commits into from Jan 14, 2020
Merged

Fixes for server command-line #1093

merged 4 commits into from Jan 14, 2020

Conversation

tailhook
Copy link
Contributor

  • rename --pidfile -> --pidfile-dir
  • more correct description of --runstate-dir

* rename `--pidfile` -> `--pidfile-dir`
* more correct description of `--runstate-dir`
@@ -468,7 +475,8 @@ def bump_rlimit_nofile() -> None:
click.option(
'--runstate-dir', type=PathPath(), default=None,
help='directory where UNIX sockets will be created '
'("/run" on Linux by default)'),
'(data-dir or {!r} by default)'
Copy link
Member

Choose a reason for hiding this comment

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

This will print or 'None' for non-production builds, which isn't very nice. I'd rewrite get_runstate_dir() as:

def _get_runstate_dir_default():
    try:
        return buildmeta.get_build_metadata_value("RUNSTATE_DIR")
    except buildmeta.MetadataError:
        return '<data-dir>`

Copy link
Member

Choose a reason for hiding this comment

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

and then:

help = f'directory where UNIX sockets and other temporary '
       f'runtime files will be placed ({_get_runstate_dir_default()} by default)'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As far as I understand the --runstate-dir is required if no RUNSTATE_DIR present in buildmeta and not a --devmode. So I'm not sure how far we want to go here with explanations.

Copy link
Member

Choose a reason for hiding this comment

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

As far as I understand the --runstate-dir is required if no RUNSTATE_DIR present in buildmeta and not a --devmode.

Buildmeta and devmode are complementary, either is supposed to be present.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay. Updated.

@1st1 1st1 merged commit c7ce551 into master Jan 14, 2020
@1st1 1st1 deleted the server_cli branch January 14, 2020 18:33
@1st1
Copy link
Member

1st1 commented Jan 14, 2020

@elprans I think you should cherry-pick this to the a2 branch.

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.

None yet

3 participants