Skip to content

Add --filter-by-connection-type flag to status command #4010

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
merged 1 commit into from
Jul 21, 2025

Conversation

aliamerj
Copy link
Contributor

@aliamerj aliamerj commented Jun 19, 2025

Describe your changes

This PR introduces a new flag --filter-by-connection-type to the status command.
It allows users to filter peers by connection type (P2P or Relayed) in both JSON and detailed views.

Input validation is added in parseFilters() to ensure proper usage, and --detail is auto-enabled if no output format is specified (consistent with other filters).

Issue ticket number and link

Resolves #3883

Stack

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor
  • Created tests that fail without the change (if possible)
  • Extended the README / documentation, if necessary

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

@aliamerj aliamerj force-pushed the status/filter-by-connection-type branch 2 times, most recently from c2e2c4a to afe35b5 Compare June 19, 2025 09:16
Copy link
Collaborator

@mlsmaycon mlsmaycon left a comment

Choose a reason for hiding this comment

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

@aliamerj I think we can limit this change to the status command only, similar to what we have for the other flags.

There is not much gain in filtering in the daemon/client

@mlsmaycon mlsmaycon self-requested a review July 3, 2025 07:08
@mlsmaycon
Copy link
Collaborator

@aliamerj I think we can limit this change to the status command only, similar to what we have for the other flags.

There is not much gain in filtering in the daemon/client

It seems like I am becoming blind... please disregard my comment

@aliamerj
Copy link
Contributor Author

aliamerj commented Jul 3, 2025

@mlsmaycon
OK :)

@aliamerj
Copy link
Contributor Author

@mlsmaycon

Is there anything should I update here ??

Copy link
Collaborator

@mlsmaycon mlsmaycon left a comment

Choose a reason for hiding this comment

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

left a small comment @aliamerj

@@ -222,6 +223,9 @@ func mapPeers(
if pbPeerState.Relayed {
connType = "Relayed"
}
if connectionTypeFilter != "" && !strings.EqualFold(connType, connectionTypeFilter) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

we should move this check into the skipDetailByFilters to align with current filter processing and output.

@aliamerj aliamerj force-pushed the status/filter-by-connection-type branch from afe35b5 to 4d78ec3 Compare July 15, 2025 19:39
Copy link

sonarqubecloud bot commented Jul 15, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
1 Accepted issue

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@aliamerj
Copy link
Contributor Author

aliamerj commented Jul 15, 2025

I just remembered why I initially didn’t include connectionTypeFilter in skipDetailByFilters.
To apply this filter, I first need to determine the connection type (connType), which depends on the Relayed flag inside the peer’s state. Since this logic didn’t exist in skipDetailByFilters yet, I needed to derive connType before calling the filter

we only want to skip a peer if its actual connection type doesn't match the filter. So instead of duplicating logic, I introduced a small getConnectionType() helper to keep things clean and consistent in both the filtering and output steps.

@aliamerj aliamerj requested a review from mlsmaycon July 15, 2025 19:47
Copy link
Collaborator

@mlsmaycon mlsmaycon left a comment

Choose a reason for hiding this comment

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

Thanks @aliamerj for the contribution

@mlsmaycon mlsmaycon merged commit 40fdeda into netbirdio:main Jul 21, 2025
31 of 32 checks passed
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.

Filter-by-status for relayed & P2P state
2 participants