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

feat!: job/step/task API functions and session log assertions #150

Merged

Conversation

jusiskin
Copy link
Contributor

@jusiskin jusiskin commented Sep 4, 2024

What was the problem/requirement? (What/Why)

There were no high-level APIs for asserting about contents in the CloudWatch session logs uploaded by the worker agent.

What was the solution? (How)

To better model the AWS Deadline Cloud resource model and APIs, I added classes and functions that allow developers to traverse from JobStepTaskSession which enables test cases using jobs with multiple steps and tasks. The Session class provides a get_session_log() method for the common use-case of collecting the session log events into memory for use in a test.

Further, the Session class has an even higher-level method (Session.assert_log_contains(...)) for asserting that a Python regular expression pattern is found in the concatenated session log in CloudWatch. In addition, this method employs exponential back-off to help account for the eventually-consistent property of logs becoming available in CloudWatch.

The exponential backoff multiplier can be configured (defaults to 300ms) and the number of backoff/retries can also be configured (default to 4). The total back-off is 4.5s using the defaults.

Another common use-case is submitting a job that has a single step and task and looking for a pattern in the task's session log. For this, a new Job.assert_single_task_log_contains(...) method was added.

What is the impact of this change?

Consumers of deadline-cloud-test-fixtures now have high-level APIs for traversing from a job through steps, tasks, and sessions, and session logs. Consumer have high-level APIs for common test use-cases to assert that a regex pattern exists in the session logs.

How was this change tested?

Unit Tests

Added unit tests for the new functionality

E2E Tests

Installed the modified deadline-cloud-test-fixtures into a deadline-cloud-worker-agent checkout dev env

  1. Ran existing Linux and Windows E2E tests and confirmed they still pass

  2. Modified this existing Linux job submission E2E test code to use the new Job.assert_single_task_log_contains(...) which makes downstream calls to all of the newly added APIs. The changed code was:

    job.assert_single_task_log_contains(
        deadline_client=deadline_client,
        logs_client=boto3.client(
            "logs",
            config=botocore.config.Config(retries={"max_attempts": 10, "mode": "adaptive"}),
        ),
        expected_pattern=rf'I am: {job_run_as_user.user}',
    )

    The test passed successfully.

Was this change documented?

No

Is this a breaking change?

Yes:

  1. in PR feedback iterations, we decided to change the existing Job.lifecycle_status attribute from a strJobLifecycleStatus enum (see feat!: job/step/task API functions and session log assertions #150 (comment))
  2. It was discovered there was a TaskStatus.UNKNOWN enum value that is not part of the AWS Deadline Cloud API so it was removed (see ref docs)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@jusiskin jusiskin added the enhancement New feature or request label Sep 4, 2024
@@ -332,7 +393,17 @@ class Job:
template: dict

name: str
lifecycle_status: str
lifecycle_status: Literal[
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we change this to use the JobLifecycleStatus enum?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed offline and the one concern is that this PR becomes a breaking change. We decided that this was an overall improvement valuable enough to justify the breaking change. Rationale:

  1. Using an enum will help test authors with IDEs that support type hints and code completion. Supporting IDEs will suggest the possible values for the enum making it easier to write tests.
  2. We are pre-1.0 and we believe that there are very few consumers of this package — likely only consumed by AWS Deadline Cloud repositories.

I've switched to the enum and marked the commit as a breaking change

Comment on lines 861 to 863
lifecycle_status: Literal[
"CREATE_COMPLETE", "UPDATE_IN_PROGRESS", "UDATE_FAILED", "UPDATE_SUCCEEDED"
]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: UDATE_FAILED -> UPDATE_FAILED

Likewise, should we change this to use StepLifecycleStatus?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switched to StepLifecycleStatus

@moorec-aws moorec-aws self-requested a review September 4, 2024 22:27
@jusiskin jusiskin force-pushed the job_step_task_apis_log_expectations branch from 5000b84 to 56affd6 Compare September 5, 2024 16:05
@jusiskin jusiskin changed the title feat: job/step/task API functions and session log assertions feat!: job/step/task API functions and session log assertions Sep 5, 2024
@jusiskin jusiskin force-pushed the job_step_task_apis_log_expectations branch from 56affd6 to e7a272b Compare September 5, 2024 16:36
@jusiskin jusiskin marked this pull request as ready for review September 5, 2024 19:40
@jusiskin jusiskin requested a review from a team as a code owner September 5, 2024 19:40
Signed-off-by: Josh Usiskin <56369778+jusiskin@users.noreply.github.com>

BREAKING CHANGE: Job.lifecycle_status changed to enum and
TaskStatus.UNKNOWN removed
@jusiskin jusiskin force-pushed the job_step_task_apis_log_expectations branch from e7a272b to 19c69e4 Compare September 5, 2024 19:53
Copy link

sonarcloud bot commented Sep 5, 2024

@jusiskin jusiskin merged commit 18f7078 into aws-deadline:mainline Sep 5, 2024
15 checks passed
@jusiskin jusiskin deleted the job_step_task_apis_log_expectations branch September 5, 2024 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants