Skip to content

Conversation

@leongdl
Copy link
Contributor

@leongdl leongdl commented Nov 15, 2024

Fixes:

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

What was the solution? (How)

What is the impact of this change?

How was this change tested?

See DEVELOPMENT.md for information on running tests.

  • Have you run the unit tests?
  • Have you run the integration tests?
  • Have you made changes to the download or asset_sync modules? If so, then it is highly recommended
    that you ensure that the docker-based unit tests pass.

Was this change documented?

  • Are relevant docstrings in the code base updated?
  • Has the README.md been updated? If you modified CLI arguments, for instance.

Does this PR introduce new dependencies?

This library is designed to be integrated into third-party applications that have bespoke and customized deployment environments. Adding dependencies will increase the chance of library version conflicts and incompatabilities. Please evaluate the addition of new dependencies. See the Dependencies section of DEVELOPMENT.md for more details.

  • This PR adds one or more new dependency Python packages. I acknowledge I have reviewed the considerations for adding dependencies in DEVELOPMENT.md.
  • This PR does not add any new dependencies.

Is this a breaking change?

A breaking change is one that modifies a public contract in a way that is not backwards compatible. See the
Public Contracts section
of the DEVELOPMENT.md for more information on the public contracts.

If so, then please describe the changes that users of this package must make to update their scripts, or Python applications.

Does this change impact security?

  • Does the change need to be threat modeled? For example, does it create or modify files/directories that must only be readable by the process owner?
    • If so, then please label this pull request with the "security" label. We'll work with you to analyze the threats.

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


Deadline Cloud offers the [Job Attachments](https://docs.aws.amazon.com/deadline-cloud/latest/userguide/storage-job-attachments.html) feature to transfer files back and forth between your workstation and AWS Deadline Cloud. For an indepth look into the Job Attachments feature, please refer to the Job Attachments [developer guide](https://docs.aws.amazon.com/deadline-cloud/latest/developerguide/submitting-files-with-a-job.html).

Where does Job Attachments come into use in the Deadline Cloud job life cycle? 1) Job Submission, 2) Moving data to workers for job execution, 3) Copying job execution output back to the Job Attachments bucket and 4) Downloading Job Outputs.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit - inconsistent cap

  1. Submission -> submission
  2. Downloading Job Outputs -> Downloading job outputs

Would it be helpful to specify the persona for the action?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, changed!

In terms of persona, I think only technical members will read this doc. It is unlikely an artist will run the CLIs unless we have a nice button click in the DCM. We can add more details if there's interest.

--exclude "*.tmp" (Optional)
```

### Uploading a manifest file to S3, useful for subsequent job submissions.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit - . -> :

--s3-manifest-prefix where/to/store/manifests \ (Optional)
--json \ (Optional)
--profile \ (Optional)
/path/for/snapshot/manifest/data-snapshot-2024-11-15T15-46-40.manifest
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this supposed to be output from the command, maybe we can make that explicit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its an (Argument), so I'll add it (Argument)

```
deadline attachment upload \
--manifests "/path/to/job/attachment.manifest" \
--s3-root-uri s3://my-queue/DeadlineCloud \
Copy link
Contributor

Choose a reason for hiding this comment

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

This is optional too, when not provided, the command would look for the default Queue and use GetQueue to retrieve JA bucket configurations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, I'll mark it as (Optional, farm-id and queue-id) alternative.

I'll keep the TLDR simple, the examples section can have both.


### Uploading files captured by Job Attachment "snapshot" to S3:
```
deadline attachment upload \
Copy link
Contributor

Choose a reason for hiding this comment

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

One of --root-dirs or --path-mapping-rules is required, but not both. They're used for determining where the attachments are for upload.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right! I need to add that.

--farm-id farm-abcdabcdabcdabcdabcdabcdabcdabcd \
--queue-id queue-11111111111111111111111111111111 \
--job-id job-22222222222222222222222222222222 \
--step-id step-33333333333333333333333333333333 \ (Optional)
Copy link
Contributor

Choose a reason for hiding this comment

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

This command should support a list of step ids for Step-Step dependencies?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, this is the step to download for, the command line internally takes care of finding the step-step dependency of the argument step.

Comment on lines +68 to +79
--farm-id farm-abcdabcdabcdabcdabcdabcdabcdabcd \
--queue-id queue-11111111111111111111111111111111 \
--job-id job-22222222222222222222222222222222 \
Copy link
Contributor

Choose a reason for hiding this comment

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

These are optional too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

farm, queue and job should not be optional? Manifest download keys on the farm, queue and job.

--job-id job-22222222222222222222222222222222 \
--step-id step-33333333333333333333333333333333 \ (Optional)
--json \ (Optional)
--profile \ (Optional)
Copy link
Contributor

Choose a reason for hiding this comment

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

When present, the credentials associated with this profile should grant access to the s3 bucket.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh great comment - super important info.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added as note for all CLIs.

- `deadline manifest diff`
Render a file folder structure tree showing new, modified and deleted files compared to a manifest.
- `deadline manifest download`
Download a manifest file from S3. Assumes Deadline Queue Crentials to access S3 bucket.
Copy link
Contributor

Choose a reason for hiding this comment

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

Crentials -> Credentials

suggestion - Assumes Deadline Queue Credentials to access S3 bucket, or use profile specified via input.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great suggestion on --profile! Added and fixed typo.,

- `deadline manifest download`
Download a manifest file from S3. Assumes Deadline Queue Crentials to access S3 bucket.
- `deadline manifest upload`
Upload a manifest file to S3. Uploaded manifests can be used for subsequent programatic job sumission. Assumes Deadline Queue Crentials to access S3 bucket.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Upload a manifest file to S3. Uploaded manifests can be used for subsequent programatic job sumission. Assumes Deadline Queue Crentials to access S3 bucket.
Upload a manifest file to S3. Uploaded manifests can be used for subsequent programmatic job submission. Assumes Deadline Queue Credentials to access S3 bucket, or use profile specified via input.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated!

@leongdl leongdl force-pushed the ja-readme branch 2 times, most recently from 06a50e8 to 7af0c9d Compare November 16, 2024 00:38
@sonarqubecloud
Copy link

@godobyte godobyte self-assigned this Dec 2, 2024
@epmog epmog added job attachments For an issue with job attachments documentation Improvements or additions to documentation labels Jan 10, 2025
@erico-aws erico-aws added the waiting-on-maintainers Waiting on the maintainers to review. label Feb 6, 2025
@epmog epmog removed the waiting-on-maintainers Waiting on the maintainers to review. label Feb 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation job attachments For an issue with job attachments

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants