-
Notifications
You must be signed in to change notification settings - Fork 51
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
MBS-11834 [Instrumentation plugin] add instrumentation.environment k8s config #1265
Conversation
5d1200c
to
6c08636
Compare
instead of implicit gradle parameters "default" config added for backward compatibility
6c08636
to
ac72363
Compare
|} | ||
| | ||
| environments { | ||
| register<com.avito.instrumentation.configuration.KubernetesViaContext>("k8sContext") { |
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.
It will be simpler for users If they can write
environments {
kubernetesViaContext("name") { }
... etc
}
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.
looks good, yes
/** | ||
* todo remove (registered for backward compatibility) | ||
*/ | ||
private fun registerDefaultEnvironment( |
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.
When do you plan to remove default
env?
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.
Right after CISteps removal, because almost every build uses instrumentation tasks, which uses default environment for now; so in a couple of days
when (environment) { | ||
is KubernetesViaCredentials -> | ||
task.kubernetesCredentials.set( | ||
KubernetesCredentials.Service( |
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.
Looks like we want to reuse those models
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'm not sure that we need to reuse models for gradle public API,
if it will be covered by proposed dsl abstraction: #1265 (comment) then yes
instead of implicit gradle parameters
"default" config added for backward compatibility