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

Add support for disabling reloader #669

Open
kou opened this issue Aug 25, 2024 · 0 comments
Open

Add support for disabling reloader #669

kou opened this issue Aug 25, 2024 · 0 comments
Labels
priority: p3 Desirable enhancement or fix. May not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@kou
Copy link

kou commented Aug 25, 2024

Is your feature request related to a problem? Please describe.

I'm an Apache Arrow developer. We're using this product for testing our GCS filesystem implementation.

We run this product by python3 -m testbench. This launches multiple processes for reloader. To shutdown these processes, we need to (1) shutdown the main process gracefully (the main process shutdowns other processes) or (2) terminate all related processes by ourselves.

Our test uses Boost.Process https://www.boost.org/libs/process to launch this product. We're using (2) because Boost.Process can't do (1) on Windows. I don't know why but (1) with Boost.Process shutdowns the launcher process (the main test process in our case) too.

We're migrating Boost.Process v2 API apache/arrow#43766 because Boost.Process v1 API is deprecated since Boost 1.86.0. Unfortunately, we can't use (2) with Boost.Process v2 API because it doesn't support process group: boostorg/process#259

If this product doesn't use multiple processes, we don't need to use (2). We can just need to terminate the main process. It can simplify our test.

Describe the solution you'd like

How about adding a new --no-use-reloader option that disables reloader something like the following?
If we disable reloader, python3 -m testbench launches only one process.

diff --git a/testbench/rest_server.py b/testbench/rest_server.py
index 9490f24..579b349 100644
--- a/testbench/rest_server.py
+++ b/testbench/rest_server.py
@@ -1174,12 +1174,14 @@ def _main():
         description="A testbench for the GCS client libraries"
     )
     parser.add_argument("--port", default=0, type=int)
+    parser.add_argument("--use-reloader", default=True,
+                        action=argparse.BooleanOptionalAction)
     args = parser.parse_args()
     serving.run_simple(
         "localhost",
         port=args.port,
         application=_run(),
-        use_reloader=True,
+        use_reloader=args.use_reloader,
         threaded=True,
     )
 

If we keep the default value of it as True, we can still use reloader by default.

Describe alternatives you've considered

Something that only uses one process.

Additional context

None.

@kou kou added priority: p3 Desirable enhancement or fix. May not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. labels Aug 25, 2024
kou added a commit to apache/arrow that referenced this issue Sep 3, 2024
### Rationale for this change

`boost/process/*.hpp` are deprecated since Boost 1.86. And it seems that it also adds backward incompatible change. We need to use `boost/process/v2/*.hpp` instead.

### What changes are included in this PR?

This introduces `arrow::util::Process` for testing. It wraps boost/process/ API. So we don't need to use boost/process/ API directly in our tests.

We still use the v1 API on Windows because the v2 API doesn't process group and we don't have a workaround for it on Windows. If GCS's testbench doesn't use multiple processes, we can use the v2 API on Windows because we don't need to use process group in our use case.

See also:
* The v2 API and process group: boostorg/process#259
* GCS's testbench and multiple processes: googleapis/storage-testbench#669

### Are these changes tested?

Yes.

### Are there any user-facing changes?

No.
* GitHub Issue: #43746

Lead-authored-by: Sutou Kouhei <[email protected]>
Co-authored-by: Sutou Kouhei <[email protected]>
Co-authored-by: Antoine Pitrou <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
mapleFU pushed a commit to mapleFU/arrow that referenced this issue Sep 3, 2024
### Rationale for this change

`boost/process/*.hpp` are deprecated since Boost 1.86. And it seems that it also adds backward incompatible change. We need to use `boost/process/v2/*.hpp` instead.

### What changes are included in this PR?

This introduces `arrow::util::Process` for testing. It wraps boost/process/ API. So we don't need to use boost/process/ API directly in our tests.

We still use the v1 API on Windows because the v2 API doesn't process group and we don't have a workaround for it on Windows. If GCS's testbench doesn't use multiple processes, we can use the v2 API on Windows because we don't need to use process group in our use case.

See also:
* The v2 API and process group: boostorg/process#259
* GCS's testbench and multiple processes: googleapis/storage-testbench#669

### Are these changes tested?

Yes.

### Are there any user-facing changes?

No.
* GitHub Issue: apache#43746

Lead-authored-by: Sutou Kouhei <[email protected]>
Co-authored-by: Sutou Kouhei <[email protected]>
Co-authored-by: Antoine Pitrou <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
zanmato1984 pushed a commit to zanmato1984/arrow that referenced this issue Sep 6, 2024
### Rationale for this change

`boost/process/*.hpp` are deprecated since Boost 1.86. And it seems that it also adds backward incompatible change. We need to use `boost/process/v2/*.hpp` instead.

### What changes are included in this PR?

This introduces `arrow::util::Process` for testing. It wraps boost/process/ API. So we don't need to use boost/process/ API directly in our tests.

We still use the v1 API on Windows because the v2 API doesn't process group and we don't have a workaround for it on Windows. If GCS's testbench doesn't use multiple processes, we can use the v2 API on Windows because we don't need to use process group in our use case.

See also:
* The v2 API and process group: boostorg/process#259
* GCS's testbench and multiple processes: googleapis/storage-testbench#669

### Are these changes tested?

Yes.

### Are there any user-facing changes?

No.
* GitHub Issue: apache#43746

Lead-authored-by: Sutou Kouhei <[email protected]>
Co-authored-by: Sutou Kouhei <[email protected]>
Co-authored-by: Antoine Pitrou <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
khwilson pushed a commit to khwilson/arrow that referenced this issue Sep 14, 2024
### Rationale for this change

`boost/process/*.hpp` are deprecated since Boost 1.86. And it seems that it also adds backward incompatible change. We need to use `boost/process/v2/*.hpp` instead.

### What changes are included in this PR?

This introduces `arrow::util::Process` for testing. It wraps boost/process/ API. So we don't need to use boost/process/ API directly in our tests.

We still use the v1 API on Windows because the v2 API doesn't process group and we don't have a workaround for it on Windows. If GCS's testbench doesn't use multiple processes, we can use the v2 API on Windows because we don't need to use process group in our use case.

See also:
* The v2 API and process group: boostorg/process#259
* GCS's testbench and multiple processes: googleapis/storage-testbench#669

### Are these changes tested?

Yes.

### Are there any user-facing changes?

No.
* GitHub Issue: apache#43746

Lead-authored-by: Sutou Kouhei <[email protected]>
Co-authored-by: Sutou Kouhei <[email protected]>
Co-authored-by: Antoine Pitrou <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p3 Desirable enhancement or fix. May not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

1 participant