Conversation
…processes to test runner
|
✅ All contributors have signed the CLA. |
| })?; | ||
| self.registry_service | ||
| .resolve_agent_type_at_deployment( | ||
| &auth.account_id(), |
There was a problem hiding this comment.
This suffers from the same problem of only behaving correctly on the user's own account
There was a problem hiding this comment.
Yes, that's a separate ticket, not trying to solve in this one (#2836)
| RegisteredAgentType agent_type = 1; | ||
| } | ||
|
|
||
| message ResolveAgentTypeAtDeploymentRequest { |
There was a problem hiding this comment.
If the plan is to allow cross-account resolution of accounts / this is ever called with a user provided account id, we need to pass the auth context here. Otherwise, the currently used system auth context is going to bypass permission checks and potentially allow users to access data they should not be able to access.
There was a problem hiding this comment.
This mirrors the existing ResolveLatestAgentTypeByNamesRequest just has a deployment_revision field
| // deploy environment to make component visible | ||
| self.deploy_environment(&component.environment_id).await?; | ||
| let deployment = self.deploy_environment(&component.environment_id).await?; | ||
| self.last_deployments |
There was a problem hiding this comment.
The auto_deployment stuff is only to allow the worker-executor style tests (which assume a component is ready as soon as its created) to work with the test dsl.
For integration tests if think an explicit user.deploy_environment(&environmentId) is clearer and gives you the deployment revision that you need.
There was a problem hiding this comment.
I like being able to copy a worker executor test to an integration test so I don't think this feature hurts
|
I have read the CLA Document and I hereby sign the CLA |
Attemps to fix all the issues that came by merging #2819 with broken checks (that were reported as green).
In details:
Agent Invocation API Fixes
Registry / Deployment
Test Infrastructure Improvements
Other