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: adding filter infrastructure + status and event filter #3247

Merged
merged 1 commit into from
Aug 11, 2024

Conversation

CodeChanning
Copy link
Contributor

@CodeChanning CodeChanning commented Jul 24, 2024

Implementing --filter flag for nerdctl events, Addresses part of #54

This change adds infrastructure for adding filters. Similar to Podman's implementation of --filter.

I also added Status and Event filter and unit tests for those.

Context:

@CodeChanning CodeChanning force-pushed the events_filter branch 6 times, most recently from c087896 to 7a2a1f2 Compare July 30, 2024 17:49
@CodeChanning CodeChanning marked this pull request as ready for review July 30, 2024 19:26
@CodeChanning
Copy link
Contributor Author

@AkihiroSuda PTAL when you get the chance

@AkihiroSuda AkihiroSuda added this to the v2.0.0 milestone Aug 5, 2024
@AkihiroSuda AkihiroSuda requested a review from fahedouch August 5, 2024 01:18
@@ -1276,7 +1276,7 @@ Flags:

- :whale: `--format`: Format the output using the given Go template, e.g, `{{json .}}`
Copy link
Contributor

Choose a reason for hiding this comment

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

HI @CodeChanning

The new flag docs can be added here. :-)

Copy link
Contributor Author

@CodeChanning CodeChanning Aug 7, 2024

Choose a reason for hiding this comment

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

Added events --filter docs. PTAL and let me know if this is sufficient. @yankay

// EventFilter for filtering events
type EventFilter func(*EventOut) bool

// Similar to podman implementation:
Copy link
Contributor

Choose a reason for hiding this comment

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

In Golang, usually, the comments should start with the Function Name. When documenting a function, the first sentence should typically start with the function's name. This format helps quickly clarify the function’s purpose.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed the comments. Let me know if that works.

@Shubhranshu153
Copy link
Contributor

@AkihiroSuda
The --sig-proxy, -a support in run and the event pr (this one) we have been adding to support dev containers on finch. Considering these are not breaking changes will it. be possible to back port these pr's to 1.7 ... or we don't plan to release any new things in 1.7 label.


isDocker := testutil.GetTarget() == testutil.Docker
if isDocker && tc.dockerSkip {
t.Skip("test is incompatible with Docker")
Copy link
Member

Choose a reason for hiding this comment

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

Could you add a comment to explain the reason?

Copy link
Member

Choose a reason for hiding this comment

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

@AkihiroSuda
Copy link
Member

1.7

I guess this is safe to cherry-pick. Feel free to open cherry-pick PRs after merging to the main branch.
cc @containerd/nerdctl-committers

Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

Thanks

@AkihiroSuda AkihiroSuda merged commit 0108a22 into containerd:main Aug 11, 2024
19 checks passed
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.

4 participants