Skip to content
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

workerManager.registerWorker: treat test-provisioner-id specially #6893

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

petemoore
Copy link
Member

Fixes: #6892

@petemoore petemoore self-assigned this Mar 8, 2024
@petemoore petemoore requested a review from a team as a code owner March 8, 2024 12:32
@petemoore petemoore requested review from lotas and matt-boris and removed request for a team March 8, 2024 12:32
@petemoore petemoore marked this pull request as draft March 8, 2024 12:58
@petemoore
Copy link
Member Author

Just realised, to get the worker config, you need the talk to the provider code, and the provider then needs to know the region, for which it needs to know what worker was created.

I might end up abandoning this... It might not be so easy to special case. For the fake worker pool definitions though, it would be sufficient to pull out the first workerConfig section from the first launchConfig, we don't need to do a region-lookup. So the test would create a fake worker pool definition like this one containing a single workerConfig in a single launch config, and registerWorker could return it.

But it only makes sense to implement this if it is a light touchpoint to the code e.g. inside a simple if statement to check the worker pool id, and pass back what it has before doing the more complex queries for other information about the worker that was created, that can be stubbed out with fake data. In the end the worker integration tests just care that they can get their config back, and the calls work to retrieve it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Worker Manager: support test payloads in registerWorker
1 participant