Remove ProviderBuilder from engine #3270
Conversation
Relates to #2845 Refactor engine to use ProviderManager. Various bits of refactoring along the way, including renaming some struct fields and variables for clarity.
| @@ -298,9 +229,7 @@ func (e *Executor) evalEntityEvent( | |||
| func (e *Executor) getEvaluator( | |||
| ctx context.Context, | |||
| inf *entities.EntityInfoWrapper, | |||
| ectx *EntityContext, | |||
There was a problem hiding this comment.
ectx was passed in to provide the project ID and the provider name. Since we no longer need provider name, and inf has the project ID - get rid of it.
|
Drop in code coverage will be undone by next PR. |
| nil, | ||
| clients.NewGitHubClientFactory(telemetry.NewNoopMetrics()), | ||
| false, | ||
| ) |
There was a problem hiding this comment.
Uhm.... is this because of the REST client cache? or is this going to be the way to instantiate the github provider? If so, I wonder how this pattern will fit when instantiating other providers such as these #2983 . Got a notion of how other client instantiations would look like?
There was a problem hiding this comment.
For posterity, we chatted about this on a call. I think we need to revisit this later - there are a bunch of things going on which may change what the ideal solution would be.
Relates to #2845
Refactor engine to use ProviderManager. Various bits of refactoring along the way, including renaming some struct fields and variables for clarity.
Summary
Provide a brief overview of the changes and the issue being addressed.
Explain the rationale and any background necessary for understanding the changes.
List dependencies required by this change, if any.
Fixes #(related issue)
Change Type
Mark the type of change your PR introduces:
Testing
Outline how the changes were tested, including steps to reproduce and any relevant configurations.
Attach screenshots if helpful.
Review Checklist: