-
Notifications
You must be signed in to change notification settings - Fork 11
feat: add dry run mode support to network and snmp discovery #140
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a dry-run mode for both SNMP and network discovery backends, allowing simulated execution without side effects.
- Added diodeDryRun and diodeDryRunOutputDir configuration fields to the relevant backend structs.
- Modified Configure and Start methods to conditionally build command-line arguments based on the dry-run setting.
- Added unit tests to verify the dry-run behavior in both discovery backends.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
agent/backend/snmpdiscovery/snmp_discovery_test.go | Added a unit test for dry-run mode for the SNMP discovery backend (note potential misnaming of the test function). |
agent/backend/snmpdiscovery/snmp_discovery.go | Updated the SNMP discovery backend to support dry-run mode with conditional command-line option construction. |
agent/backend/networkdiscovery/network_discovery_test.go | Added a unit test for dry-run mode for the network discovery backend. |
agent/backend/networkdiscovery/network_discovery.go | Updated the network discovery backend to support dry-run mode with conditional command-line argument construction. |
Comments suppressed due to low confidence (1)
agent/backend/snmpdiscovery/snmp_discovery_test.go:205
- The test function name suggests it is for network discovery, but this file is for the SNMP discovery backend; consider renaming it to TestSNMPDiscoveryBackendDryRun for clarity.
func TestNetworkDiscoveryBackendDryRun(t *testing.T) {
Go test coverage
Total coverage: 58.0% |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
🎉 This PR is included in version 2.3.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This pull request introduces a "dry-run" mode to the
networkDiscoveryBackend
andsnmpDiscoveryBackend
components, enabling safer testing by simulating operations without executing them. It also adds corresponding configurations and unit tests to ensure the functionality works as expected.Backend Enhancements:
diodeDryRun
anddiodeDryRunOutputDir
fields tonetworkDiscoveryBackend
andsnmpDiscoveryBackend
structs to support dry-run mode. (agent/backend/networkdiscovery/network_discovery.go
: [1]agent/backend/snmpdiscovery/snmp_discovery.go
: [2]Configure
methods to populate dry-run-related fields from configuration and handle optional parameters. (agent/backend/networkdiscovery/network_discovery.go
: [1]agent/backend/snmpdiscovery/snmp_discovery.go
: [2]Start
methods to adjust command-line arguments based on dry-run mode, ensuring safe execution and proper logging. (agent/backend/networkdiscovery/network_discovery.go
: [1]agent/backend/snmpdiscovery/snmp_discovery.go
: [2]Testing Additions:
networkDiscoveryBackend
andsnmpDiscoveryBackend
, verifying configuration, startup behavior, and command-line arguments. (agent/backend/networkdiscovery/network_discovery_test.go
: [1]agent/backend/snmpdiscovery/snmp_discovery_test.go
: [2]