Skip to content

Run init task before updateing service #273

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

Closed

Conversation

chiragrajk
Copy link

Issue #, if available:

Description of changes:

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

@chiragrajk chiragrajk marked this pull request as draft November 10, 2021 13:35
@chiragrajk chiragrajk force-pushed the feature/enable-init-tasks branch 23 times, most recently from 7264b76 to 708d38e Compare November 16, 2021 09:56
@chiragrajk chiragrajk force-pushed the feature/enable-init-tasks branch from 708d38e to 6ae0b08 Compare November 25, 2021 15:26
@chiragrajk chiragrajk force-pushed the feature/enable-init-tasks branch from 6ae0b08 to 87805ba Compare October 10, 2022 08:26
@chiragrajk chiragrajk force-pushed the feature/enable-init-tasks branch 3 times, most recently from bce0b50 to 840e0d5 Compare January 20, 2023 13:30
@chiragrajk chiragrajk force-pushed the feature/enable-init-tasks branch from 840e0d5 to 632654e Compare January 25, 2024 16:21
@chiragrajk chiragrajk force-pushed the feature/enable-init-tasks branch from 632654e to 41d6767 Compare February 19, 2024 09:52
* add desired count

* add desired count

* add desired count

* fix test for desired count

* small change

* update dist/index.js

* address feedback @iamhopaul123

* update dist/index.js

---------

Co-authored-by: Adithya Kolla <[email protected]>
@chiragrajk chiragrajk force-pushed the feature/enable-init-tasks branch from 41d6767 to 1381c77 Compare April 9, 2024 14:31
@chiragrajk chiragrajk force-pushed the feature/enable-init-tasks branch from 1381c77 to 99ebbbe Compare April 9, 2024 14:33
@amazreech
Copy link
Contributor

amazreech commented May 10, 2024

Hi @chiragrajk,

Thank you for your contribution, apologies on the delay. We will be working to review the changes in the Pull Request. In the meantime please ensure that below steps, if not already completed, are taken care of in your Pull Request:

  1. Verify if PR follows semantic pull request conventions.
    Example: Update the PR title to follow conventions feat: Run init task before ...

  2. Resolve any conflicts on the PR

  3. Please run npm run package command to update dist/ folder with latest dependencies.

  4. Please update PR descirption with any additional context on the code.

Thank you!


mockRunTasks.mockImplementation(() => {
return {
promise() {
Copy link
Contributor

Choose a reason for hiding this comment

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

The mock needs to be updated as the action is now using JS SDK v3

Refer: #529

Copy link
Contributor

@trivikr trivikr left a comment

Choose a reason for hiding this comment

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

Provided diff for transforming JS SDK v2 APIs to v3.

Refs: #529

Comment on lines +31 to +46
const runTaskResponse = await ecs.runTask({
startedBy: startedBy,
cluster: clusterName,
taskDefinition: taskDefArn,
enableExecuteCommand: true,
overrides: {
containerOverrides: [
{
name: containerName,
command: initTaskCommand.split(' ')
}
]
},
launchType: 'FARGATE',
networkConfiguration: networkConfiguration
}).promise();
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
const runTaskResponse = await ecs.runTask({
startedBy: startedBy,
cluster: clusterName,
taskDefinition: taskDefArn,
enableExecuteCommand: true,
overrides: {
containerOverrides: [
{
name: containerName,
command: initTaskCommand.split(' ')
}
]
},
launchType: 'FARGATE',
networkConfiguration: networkConfiguration
}).promise();
const runTaskResponse = await ecs.runTask({
startedBy: startedBy,
cluster: clusterName,
taskDefinition: taskDefArn,
enableExecuteCommand: true,
overrides: {
containerOverrides: [
{
name: containerName,
command: initTaskCommand.split(' ')
}
]
},
launchType: 'FARGATE',
networkConfiguration: networkConfiguration
});

Comment on lines +79 to +90
const maxAttempts = (waitForMinutes * 60) / WAIT_DEFAULT_DELAY_SEC;

core.info('Waiting for tasks to stop');

const waitTaskResponse = await ecs.waitFor('tasksStopped', {
cluster: clusterName,
tasks: taskArns,
$waiter: {
delay: WAIT_DEFAULT_DELAY_SEC,
maxAttempts: maxAttempts
}
}).promise();
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
const maxAttempts = (waitForMinutes * 60) / WAIT_DEFAULT_DELAY_SEC;
core.info('Waiting for tasks to stop');
const waitTaskResponse = await ecs.waitFor('tasksStopped', {
cluster: clusterName,
tasks: taskArns,
$waiter: {
delay: WAIT_DEFAULT_DELAY_SEC,
maxAttempts: maxAttempts
}
}).promise();
core.info('Waiting for tasks to stop');
const waitTaskResponse = await waitUntilTasksStopped({
client: ecs,
minDelay: WAIT_DEFAULT_DELAY_SEC,
maxWaitTime: waitForMinutes * 60,
}, {
cluster: clusterName,
tasks: taskArns,
});

Comment on lines +98 to +101
const describeResponse = await ecs.describeTasks({
cluster: clusterName,
tasks: taskArns
}).promise();
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
const describeResponse = await ecs.describeTasks({
cluster: clusterName,
tasks: taskArns
}).promise();
const describeResponse = await ecs.describeTasks({
cluster: clusterName,
tasks: taskArns
});

s3cube pushed a commit that referenced this pull request Jul 16, 2024
Bumps [eslint](https://github.com/eslint/eslint) from 8.50.0 to 8.51.0.
- [Release notes](https://github.com/eslint/eslint/releases)
- [Changelog](https://github.com/eslint/eslint/blob/main/CHANGELOG.md)
- [Commits](eslint/eslint@v8.50.0...v8.51.0)

---
updated-dependencies:
- dependency-name: eslint
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@amazreech
Copy link
Contributor

Changes to run init task has been merged via PR #340. Closing out this PR.

@amazreech amazreech closed this Aug 1, 2024
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.

4 participants