-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[AGENTRUN-782] Set disk and network v2 checks to default for windows and linux #42003
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
Open
jeremy-hanna
wants to merge
11
commits into
main
Choose a base branch
from
jeremy.hanna/default-check-configs
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+59
−29
Open
Changes from 6 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
a4bd363
Default to use the networkv2 checks for Windows and Linux in config s…
jeremy-hanna 07da7cf
Merge branch 'main' into jeremy.hanna/default-check-configs
jeremy-hanna 73f9954
Default to use the networkv2 checks for Windows and Linux in config s…
jeremy-hanna 0d0800e
Add enhancement CHANGELOG entry for go core checks
jeremy-hanna 25bf10d
Fix failing unit test around new default values
jeremy-hanna 0b60467
Merge branch 'main' into jeremy.hanna/default-check-configs
jeremy-hanna e1e140c
Set python_lazy_loading to false for e2e tests and make status reflec…
jeremy-hanna 1a8a75a
Merge branch 'main' into jeremy.hanna/default-check-configs
jeremy-hanna 7f50dc6
Fix linting error
jeremy-hanna e89899a
Updates from PR comments
jeremy-hanna 11d7fc1
Merge branch 'main' into jeremy.hanna/default-check-configs
jeremy-hanna File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
17 changes: 17 additions & 0 deletions
17
releasenotes/notes/enable-v2-checks-by-default-ff71cdcb4455bd43.yaml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
# Each section from every release note are combined when the | ||
# CHANGELOG.rst is rendered. So the text needs to be worded so that | ||
# it does not depend on any information only available in another | ||
# section. This may mean repeating some details, but each section | ||
# must be readable independently of the other. | ||
# | ||
# Each section note must be formatted as reStructuredText. | ||
--- | ||
enhancements: | ||
- | | ||
Enable the Go disk and network core checks by default for Windows and Linux. | ||
These are direct ports of the existing Python disk and network checks and allow | ||
the Python runtime to be lazy loaded when other integrations are enabled. It | ||
can be disabled with setting ``use_diskv2_check`` and ``use_networkv2_check`` | ||
respectively along with the loader in your configuration to use the Python | ||
version. | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
we are in diskv2 folder, don't we want to set it to true ?
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.
use_diskv2_check
takes precedence overdisk_check.use_core_loader
in the codeIe. if
use_diskv2_check: true
then we actually use the Go v2 check instead of the Python check, no matter the value ofdisk_check.use_core_loader
Which is quite counter intuitive though 😄
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.
Thanks for the details 🙇🏻
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.
I think your comment still reveals an issue, we're testing something which can't happen here: if
use_diskv2_check
isfalse
then the v2 code is unreachable, so there is no point in checking (and testing) that hereI think we can just remove the test and the whole condition at the beginning of
Configure
in diskv2 (btw there is no equivalent in networkv2)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.
this is testing that an error is thrown when trying to initialize the diskv2 check when it shouldn't be used. It maybe should just be deleted instead