-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Remove unused environment from anomaly detector classes #54399
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
|
Pinging @elastic/ml-core (:ml) |
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.
Intellij flagged this as a get on an optional without a isPresent check which is a very valid warning. In practice I don't think we could hit an NPE here but it shows the value in the inspections if you can keep the noise level down.
dimitris-athanasiou
left a comment
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 Nice! Just left an optional suggestion.
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.
nit: make it a single liner using ternary?
przemekwitek
left a comment
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
Following #54371 I realised the
Environmentvariable being passed in to the various AD managers is unused.This takes removes the environment var and some minor tidying up to clean up Intellij inspection warnings