feat: support connecting to a pocket-ic server running inside a Docker container.#4271
Draft
vincent-dfinity wants to merge 13 commits intomasterfrom
Draft
feat: support connecting to a pocket-ic server running inside a Docker container.#4271vincent-dfinity wants to merge 13 commits intomasterfrom
vincent-dfinity wants to merge 13 commits intomasterfrom
Conversation
…stead of 0.0.0.0 for pocket-ic server.
…the temporary frontend address solution.
vincent-dfinity
changed the title
mraszyk
reviewed
Jun 18, 2025
Comment on lines
+540
to
+541
| if std::process::Command::new("docker") | ||
| .args(["--version"]) |
Contributor
There was a problem hiding this comment.
Even if the command is available, docker might still fail if the daemon is not running.
mraszyk
reviewed
Jun 18, 2025
| } | ||
| } | ||
| }; | ||
| println!("port: {}", port); |
mraszyk
reviewed
Jun 18, 2025
| "-p", | ||
| format!("{}:{}", bind_address.port(), bind_address.port()).as_str(), | ||
| "-p", | ||
| "8081:8081", |
Contributor
There was a problem hiding this comment.
This assumes a fixed port on the host without an option to override.
mraszyk
reviewed
Jun 18, 2025
| bind_address = convert_to_docker_bind_address(bind_address); | ||
| cmd.args([ | ||
| "-p", | ||
| format!("{}:{}", bind_address.port(), bind_address.port()).as_str(), |
Contributor
There was a problem hiding this comment.
Inside the container a fixed port could be used for simplicity.
mraszyk
reviewed
Jun 18, 2025
Comment on lines
+124
to
+125
| let output = std::process::Command::new("docker") | ||
| .args(["exec", container_id.as_str(), "cat", "pocket-ic-port"]) |
Contributor
There was a problem hiding this comment.
Why is the (host) port retrieved from the docker container? I'd expect the host port to be stored on the host and mapped to a (fixed) port used in the docker container.
mraszyk
reviewed
Jun 18, 2025
| "8081:8081", | ||
| ]); | ||
| cmd.args(["-d", docker_image.as_str()]); | ||
| cmd.stdout(std::process::Stdio::piped()); |
Contributor
There was a problem hiding this comment.
I'd also expect the -v option to be used in order to mount the state to be persisted.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The pr supports connecting to a pocket-ic server running inside a Docker container.
Fixes # (issue)
How Has This Been Tested?
Manually tested so far by starting, deploying, and stopping.
Checklist: