Allow server process to start without detaching #11319

f1sherman · 2022-12-22T22:11:04Z

Add a --no-detach option to --start-server and --restart-server. This makes it easy to run the RuboCop server process from within Docker, where detaching the process causes the container to exit.

Fixes #11188

Before submitting the PR make sure the following are checked:

The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
Wrote good commit messages.
Commit message starts with [Fix #issue-number] (if the related issue exists).
Feature branch is up-to-date with master (if not - rebase it).
Squashed related commits together.
Added tests.
Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.

bbatsov · 2022-12-31T07:26:03Z

@f1sherman I see that a bunch of CI jobs are currently failing.

f1sherman · 2022-12-31T15:06:35Z

@bbatsov sorry about that! CI is fixed now. I was a little confused about the below offense since I didn't change that file, so I fixed that one in a separate commit.

lib/rubocop/cop/style/redundant_string_escape.rb:61:42: C: [Corrected] Performance/RedundantStringChars: Use [-1] instead of chars.last.
          format(MSG, char: range.source.chars.last)
                                         ^^^^^^^^^^

koic · 2022-12-31T15:19:42Z

@bbatsov sorry about that! CI is fixed now. I was a little confused about the below offense since I didn't change that file, so I fixed that one in a separate commit.

No need for that the separated commit because this offense has already been resolved on the master branch:
63e8377

Can you rebase with the latest master branch?

koic · 2022-12-31T15:24:42Z

I wonder whether the term "daemon" is appropriate as I remember the conversation below:
#10706 (comment)

f1sherman · 2022-12-31T15:25:00Z

Can you rebase with the latest master branch?

Done!

f1sherman · 2022-12-31T15:26:11Z

I wonder whether the term "daemon" is appropriate

I'm fully open to suggestions! Would --no-background be preferable?

mculp · 2023-01-03T02:17:45Z

The code uses demonize -- at least you spelled it correctly 😂

mculp · 2023-01-03T06:11:13Z

Thank you @f1sherman. Another use case for this: DarthSim/overmind#143

mculp · 2023-01-03T06:19:06Z

lib/rubocop/server/cli.rb

      ].freeze
      EXCLUSIVE_OPTIONS = (SERVER_OPTIONS - %w[--server --no-server]).freeze
+      NO_DAEMONIZE_OPTIONS = %w[--server --start-server].freeze


What do you think about --restart-server here? Example, if you had started it daemonized and wanted to restart it in foreground?

Also, should we consider what the output should be if it's not daemonized?

Should it be --debug output?
Should it show simple messages like "processing 5510 files" every time Rubocop is run?

Thanks for the feedback! I've made an update to allow --no-daemonize to be used with --restart-server.

I don't have a very strong opinion about the process output. That said, with the new year starting I have less time to polish this PR up so I could certainly use an assist or strong point in the right direction on how to make the changes you're looking for if you'd be so kind. Thank you!

I don't have strong opinions either, would rather just get this merged and then we can do a follow-up for the output

mculp · 2023-01-11T00:52:21Z

@bbatsov @koic Looks like CI is passing now.

bbatsov · 2023-02-01T06:11:21Z

I'm fully open to suggestions! Would --no-background be preferable?

I think the most common name for such an option is usually "--nodetach" or "-no-detach".

bbatsov · 2023-02-01T06:14:11Z

(it seems that even Docker favours this terminology)

bbatsov · 2023-02-01T06:15:34Z

docs/modules/ROOT/pages/usage/server.adoc

@@ -57,6 +57,12 @@ RuboCop version incompatibility found, RuboCop server restarting...
 RuboCop server starting on 127.0.0.1:60665.
 ```

+If you would like to start the server without daemonizing the process, which can be useful when running within Docker, you can pass the `--no-daemonize` option.


without daemonizing -> in the foreground

bbatsov · 2023-02-01T06:15:41Z

docs/modules/ROOT/pages/usage/server.adoc

@@ -92,6 +98,9 @@ These are the command-line options for server operations:

 | `--server-status`
 | Show server status.
+
+| `--no-daemonize`
+| Do not daemonize the server process.


bbatsov · 2023-02-01T06:16:09Z

lib/rubocop/server/cli.rb

      # rubocop:disable Metrics/CyclomaticComplexity, Metrics/MethodLength:
-      def run_command(server_command)
+      def run_command(server_command, daemonize:)


daemonize -> detach

Add a `--no-detach` option to `--start-server`. This makes it easy to run the RuboCop server process from within Docker, where detaching the process causes the container to exit.

f1sherman · 2023-02-01T14:54:19Z

@bbatsov thanks for the feedback, we should be all set once CI finishes 🤞

f1sherman changed the title ~~[Fix #11188]~~ Allow server process to start without daemonizing Dec 31, 2022

f1sherman force-pushed the no-daemonize-server branch from f9f5b2b to 8b102d2 Compare December 31, 2022 15:00

f1sherman force-pushed the no-daemonize-server branch from 8b102d2 to 7012d4c Compare December 31, 2022 15:24

f1sherman force-pushed the no-daemonize-server branch from 7012d4c to 42f5b2b Compare December 31, 2022 15:28

mculp mentioned this pull request Jan 3, 2023

Gracefully handle backgrounded processes DarthSim/overmind#143

Closed

mculp reviewed Jan 3, 2023

View reviewed changes

f1sherman force-pushed the no-daemonize-server branch from 42f5b2b to 184e33e Compare January 3, 2023 16:03

bbatsov reviewed Feb 1, 2023

View reviewed changes

f1sherman force-pushed the no-daemonize-server branch from bd17f5d to be74773 Compare February 1, 2023 14:33

[Fix rubocop#11188]

888c65d

Add a `--no-detach` option to `--start-server`. This makes it easy to run the RuboCop server process from within Docker, where detaching the process causes the container to exit.

f1sherman force-pushed the no-daemonize-server branch from 0a1a87a to 888c65d Compare February 1, 2023 14:40

f1sherman changed the title ~~Allow server process to start without daemonizing~~ Allow server process to start without detaching Feb 1, 2023

bbatsov merged commit 5230ef2 into rubocop:master Feb 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow server process to start without detaching #11319

Allow server process to start without detaching #11319

f1sherman commented Dec 22, 2022 •

edited

bbatsov commented Dec 31, 2022

f1sherman commented Dec 31, 2022

koic commented Dec 31, 2022 •

edited

koic commented Dec 31, 2022 •

edited

f1sherman commented Dec 31, 2022

f1sherman commented Dec 31, 2022

mculp commented Jan 3, 2023

mculp commented Jan 3, 2023

mculp Jan 3, 2023

f1sherman Jan 3, 2023

mculp Jan 9, 2023

mculp commented Jan 11, 2023

bbatsov commented Feb 1, 2023

bbatsov commented Feb 1, 2023

bbatsov Feb 1, 2023

bbatsov Feb 1, 2023

bbatsov Feb 1, 2023

f1sherman commented Feb 1, 2023

Allow server process to start without detaching #11319

Allow server process to start without detaching #11319

Conversation

f1sherman commented Dec 22, 2022 • edited

bbatsov commented Dec 31, 2022

f1sherman commented Dec 31, 2022

koic commented Dec 31, 2022 • edited

koic commented Dec 31, 2022 • edited

f1sherman commented Dec 31, 2022

f1sherman commented Dec 31, 2022

mculp commented Jan 3, 2023

mculp commented Jan 3, 2023

mculp Jan 3, 2023

Choose a reason for hiding this comment

f1sherman Jan 3, 2023

Choose a reason for hiding this comment

mculp Jan 9, 2023

Choose a reason for hiding this comment

mculp commented Jan 11, 2023

bbatsov commented Feb 1, 2023

bbatsov commented Feb 1, 2023

bbatsov Feb 1, 2023

Choose a reason for hiding this comment

bbatsov Feb 1, 2023

Choose a reason for hiding this comment

bbatsov Feb 1, 2023

Choose a reason for hiding this comment

f1sherman commented Feb 1, 2023

f1sherman commented Dec 22, 2022 •

edited

koic commented Dec 31, 2022 •

edited

koic commented Dec 31, 2022 •

edited