test: Adjust workers for start service disabled#185
test: Adjust workers for start service disabled#185hindleym merged 5 commits intoaws-deadline:mainlinefrom
Conversation
Signed-off-by: Church Hindley <59745380+hindleym@users.noreply.github.com>
|
These |
| else: | ||
| raise TimeoutError | ||
|
|
||
| def wait_until_worker_stopping( |
There was a problem hiding this comment.
nit, can we make wait_until_stopped and wait_until_worker_stopping share a function? The function wait_until_X can just pass in the state to wait for.
There was a problem hiding this comment.
Simplified these to wait_until_desired_worker_status
| ) | ||
| assert cmd_result.exit_code == 0, f"Failed to configure Worker agent: {cmd_result}" | ||
| LOG.info("Successfully configured Worker agent") | ||
| LOG.info("Sending SSM Command to check if Worker Agent is running") |
There was a problem hiding this comment.
Any reason why this check was moved below? I'm not sure of the code all path.
There was a problem hiding this comment.
What's the diff here for _start_worker_agent vs start_worker_service for your testing?
There was a problem hiding this comment.
Similarly to how we handle this with Linux here.
If start-service is set to False, we want to make sure that the checks for the service to be running don't occur when installing the agent.
_start_worker_agent() is part of the base class and is called when the worker is created with .start().
start_worker_service() allows us to start the worker at a later time, using the installed configuration.
There was a problem hiding this comment.
Does it sense to rename _start_worker_agent to _setup_worker_agent or something like that to avoid future confusion?
hindleym
force-pushed
the
hindleym-check-start-service-in-worker
branch
from
8d18d6d to
0ffbb3a
Compare
Signed-off-by: Church Hindley <59745380+hindleym@users.noreply.github.com>
Signed-off-by: Church Hindley <59745380+hindleym@users.noreply.github.com>
| LOG.exception(f"Failed to delete worker: {error}") | ||
| raise | ||
|
|
||
| def wait_until_stopped( |
There was a problem hiding this comment.
This is an API that is consumed in other packages, please don't just change the name.
What I'm suggesting is:
def wait_until_stopped():
wait_until_desired_worker_status(desired_status="STOPPED"
Signed-off-by: Church Hindley <59745380+hindleym@users.noreply.github>
|
| ) | ||
| assert cmd_result.exit_code == 0, f"Failed to configure Worker agent: {cmd_result}" | ||
| LOG.info("Successfully configured Worker agent") | ||
| LOG.info("Sending SSM Command to check if Worker Agent is running") |
There was a problem hiding this comment.
Does it sense to rename _start_worker_agent to _setup_worker_agent or something like that to avoid future confusion?
|
|
||
| def start_worker_service(self): | ||
| LOG.info("Sending command to start the Worker Agent service") | ||
| LOG.info("Sending command to start the Worker Agent service - Added comma") |
There was a problem hiding this comment.
Did we mean to add this "- Added comma" here?
There was a problem hiding this comment.
No, I meant to get rid of that. I'll make sure to get that with my next set of changes.
3 participants
What was the problem/requirement? (What/Why)
get_worker_id(), even if the worker wasn't startedWhat was the solution? (How)
_start_worker_agent()functionstart_worker_service()functionget_worker_id()What is the impact of this change?
Improves test framework to handle the start-service flag
How was this change tested?
Verified locally against the Windows and Linux E2E tests
Was this change documented?
No
Is this a breaking change?
No
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.