-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
chore(batch): update Batch README.md #6931
Conversation
I see there is a lot of commits. I will do a rebase shortly 👍 |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
@stephnr This is awesome! But there seem to be a lot of unrelated changes, for example, the stability has changed from Can you have another look and clean this up? Thanks. |
That is the intention here. I worked on an earlier PR here to provide a stable 1.0 release of the aws-batch CDK library. This PR is to update the README. |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
The process for marking the package as stable is a little more involved @stephnr :) Leave it as "Experimental" for this PR. |
@stephnr notice the Build is still failing. Have a look? |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
@iliapolo could you help me here? I have the stability banner there in the README file but its still complaining. |
@stephnr You only have the stability badge, not the banner. You can fix this problem by simply running In addition, the changes to the integ expectation file should be reverted as well. It doesn't make sense to introduce changes to the expectations without introducing changes to the code, and I imagine this will be the next thing CodeBuild will complain about. |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
@stephnr I see that you made some changes but i fear you were using an outdated version of Here is what the stability banner should look like:
And it will also fix the I will start commenting on the actual README content in the meantime :) |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
@iliapolo hey! Sorry for all the delays and issues. I have fixed things up and would love your approval/review after these merge updates are finished up. |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @stephnr - Thanks for this great addition! we desperately needed it.
This is quite a lot of important content, so naturally I didn't skim on comments :). Please have a look and let me know what you think. Hopefully now that the build stuff are worked out we can get this merged fairly quickly.
Thanks!
@@ -13,39 +14,271 @@ | |||
--- | |||
<!--END STABILITY BANNER--> | |||
|
|||
This module is part of the [AWS Cloud Development Kit](https://github.com/aws/aws-cdk) project. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please leave this as is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@iliapolo I will add this back in directly underneath the banner (if thats ok). Otherwise, we can rearrange it.
}); | ||
``` | ||
|
||
In this example, both job queues share the same set of compute environments but our `highPrioQueue` will have its jobs assigned first. Even though our other queue might have jobs waiting to be assigned, they will be ignored until our `highPrioQueue` has no jobs waiting for assignment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even though our other queue might have jobs waiting to be assigned, they will be ignored until our
highPrioQueue
has no jobs waiting for assignment.
Relates to what i noted above...Are you sure this true? It seems counter intuitive. If i have one job in each queue, and 5 compute environments available that can serve both jobs, won't they run simultaneously?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is actually true... queues do use multiple environments if linked 🤔 . I will go ahead and modify/rework this then.
Below is an example: | ||
|
||
```ts | ||
const jobQueue = JobQueue.fromJobQueueArn(this, 'imported-job-queue', 'arn:aws:batch:us-east-1:555555555555:job-queue/High-Prio-Queue'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const jobQueue = JobQueue.fromJobQueueArn(this, 'imported-job-queue', 'arn:aws:batch:us-east-1:555555555555:job-queue/High-Prio-Queue'); | |
const jobQueue = batch.JobQueue.fromJobQueueArn(this, 'imported-job-queue', 'arn:aws:batch:us-east-1:555555555555:job-queue/High-Prio-Queue'); |
Below is an example: | ||
|
||
```ts | ||
const job = JobDefinition.fromJobDefinitionArn(this, 'imported-job-definition', 'arn:aws:batch:us-east-1:555555555555:job-definition/my-job-definition'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const job = JobDefinition.fromJobDefinitionArn(this, 'imported-job-definition', 'arn:aws:batch:us-east-1:555555555555:job-definition/my-job-definition'); | |
const job = batch.JobDefinition.fromJobDefinitionArn(this, 'imported-job-definition', 'arn:aws:batch:us-east-1:555555555555:job-definition/my-job-definition'); |
|
||
### Using a local Docker project | ||
|
||
Below is an example of how you can create a Batch Job Definition from a local Docker projects on deploy of your CDK project: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Below is an example of how you can create a Batch Job Definition from a local Docker projects on deploy of your CDK project: | |
Below is an example of how you can create a Batch Job Definition from a local Docker application. |
A Batch Job definition helps AWS Batch understand important details about how to run your application in the scope of a Batch Job. This involves key information like resource requirements, what containers to run, how the compute environment should be prepared, and more. Below is a simple example of how to create a job definition: | ||
|
||
```ts | ||
const repo = new ecr.Repository(stack, 'batch-job-repo'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe use an existing repo as an example? It more clearly shows a link to a specific image:
const repo = new ecr.Repository(stack, 'batch-job-repo'); | |
const repo = ecr.Repository.fromRepositoryName(stack, 'batch-job-repo', 'todo-list'); |
This PR has not received a response in a while. If you want to keep this issue open, please leave a comment below and auto-close will be canceled. |
dont close |
I’ll try to get to these comments over the weekend!
…On May 21, 2020, 16:31 +0200, Eli Polonsky ***@***.***>, wrote:
dont close
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
@stephnr any reason you closed this? |
Hmm. strange...ok ill move over to that one. |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Commit Message
chore(batch): update Batch README.md (#6931)
This should be the final docs update based on the recent changes to:
End Commit Message
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license