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

feat(service): Add docker service ls --filter option #561

Merged

Conversation

fuentes73
Copy link
Contributor

docker service ls --filter option

@fuentes73 fuentes73 force-pushed the add-service-filter branch 2 times, most recently from c9c5e1e to c54749c Compare March 8, 2024 12:18
Copy link
Owner

@gabrieldemarmiesse gabrieldemarmiesse left a comment

Choose a reason for hiding this comment

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

Many thanks for the pull, request, this is a cool feature. We just need a few changes and it will good to merge :)

@@ -353,13 +353,14 @@ def logs(
else:
return "".join(x[1].decode() for x in iterator)

def list(self) -> List[Service]:
def list(self, filters: Dict[str, str] = {}) -> List[Service]:

Choose a reason for hiding this comment

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

Can you update the docstring with an example? Thanks!

@@ -45,6 +45,25 @@ def test_get_list_of_services(docker_client: DockerClient):
assert [my_service] == list_of_services


@pytest.mark.usefixtures("swarm_mode")
def test_filters(docker_client: DockerClient):

Choose a reason for hiding this comment

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

Can you try to filter with a label that does not exist and verify that no services are returned? That would ensure that the function is correctly removing services from the result.

@fuentes73
Copy link
Contributor Author

@gabrieldemarmiesse thanks for your quick feedback. Done, let me know if everything is ok.

Copy link
Owner

@gabrieldemarmiesse gabrieldemarmiesse left a comment

Choose a reason for hiding this comment

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

Many thanks!

auto-merge was automatically disabled March 9, 2024 09:53

Head branch was pushed to by a user without write access

@fuentes73
Copy link
Contributor Author

@gabrieldemarmiesse i've had to rebase because it wasn't up to date. Can you enable auto-merge again?

@gabrieldemarmiesse gabrieldemarmiesse merged commit e45f0cd into gabrieldemarmiesse:master Mar 9, 2024
35 checks passed
@fuentes73 fuentes73 deleted the add-service-filter branch March 9, 2024 10:13
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.

2 participants