Skip to content

Add signal query argument for Restart Container command #2186

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

Merged

Conversation

dmmax
Copy link
Contributor

@dmmax dmmax commented Aug 21, 2023

Linked to the issue: #2123

@dmmax dmmax requested a review from a team as a code owner August 21, 2023 05:11
@eddumelendez
Copy link
Member

Can you add a test, please?

@eddumelendez eddumelendez added this to the next milestone Aug 21, 2023
@dmmax
Copy link
Contributor Author

dmmax commented Aug 21, 2023

Hi @eddumelendez

I added an integration test that covers the new functionality.

I also had to add new API versions to the RemoteApiVersion. Let me know if it's ok for you

BTW The test com.github.dockerjava.cmd.ListContainersCmdIT.testStatusFilter is failing for me as well (on the main branch) on the local machine and only increasing the timeout for stopping the container helps. Is it expected behavior?

@dmmax
Copy link
Contributor Author

dmmax commented Sep 4, 2023

Hi @eddumelendez
Can you have a look again into the PR? 🙏

.withRegistryUrl("https://index.docker.io/v1/")
.build();
try (DockerClient dockerClient = createDockerClient(dockerClientConfig)) {
if (!getVersion(dockerClient).isGreaterOrEqual(RemoteApiVersion.VERSION_1_42)) {
Copy link
Member

Choose a reason for hiding this comment

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

I think we can use Assume instead.

@eddumelendez eddumelendez merged commit fd5da16 into docker-java:main Oct 27, 2023
@eddumelendez
Copy link
Member

Thanks for your contribution, @dmmax !

@dmmax dmmax deleted the add_signal_to_restart_container_cmd branch October 29, 2023 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants