Skip to content

Commit

Permalink
Merge pull request #68 from openhie/CU-86by2x2u5-concurrent-package-a…
Browse files Browse the repository at this point in the history
…ctions

Allow concurrent actions on packages
  • Loading branch information
rcrichton authored Apr 19, 2024
2 parents f1228be + 289e1cf commit 6277c2b
Show file tree
Hide file tree
Showing 12 changed files with 3,231 additions and 662 deletions.
25 changes: 25 additions & 0 deletions .github/workflows/test-instant.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
name: Run Instant Tests

on:
pull_request:
branches: [main]
push:
branches: [main]

jobs:
instant-tests:
runs-on: ubuntu-latest
steps:
- name: Checkout code
uses: actions/checkout@v3

- name: Setup Node.js
uses: actions/setup-node@v3
with:
node-version: '20'

- name: Install dependencies
run: yarn install

- name: Run Tests
run: yarn test
3 changes: 3 additions & 0 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,7 @@ ADD instant.ts .
# add util function scripts
ADD utils ./utils

# add schema
ADD schema ./schema

ENTRYPOINT [ "yarn", "instant" ]
6 changes: 6 additions & 0 deletions babel.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
module.exports = {
presets: [
['@babel/preset-env', { targets: { node: 'current' } }],
'@babel/preset-typescript'
]
}
1 change: 1 addition & 0 deletions cli/src/cmd/flags/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,5 @@ func setCommonActionFlags(cmd *cobra.Command) {
flags.StringSliceVar(&state.EnvFiles, "env-file", nil, "env file")
flags.StringVar(&state.ConfigFile, "config", "", "config file (default is $WORKING_DIR/config.yaml)")
flags.StringSliceP("env-var", "e", nil, "Env var(s) to set or overwrite")
flags.StringP("concurrency", "", "", "The concurrency level to use for executing actions on packages (default 5)")
}
4 changes: 4 additions & 0 deletions cli/src/core/parse/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ func GetInstantCommand(packageSpec core.PackageSpec) []string {
instantCommand = append(instantCommand, "--only")
}

if packageSpec.Concurrency != "" {
instantCommand = append(instantCommand, "--concurrency", packageSpec.Concurrency)
}

instantCommand = append(instantCommand, packageSpec.Packages...)

for _, customPackage := range packageSpec.CustomPackages {
Expand Down
5 changes: 5 additions & 0 deletions cli/src/core/parse/packageSpec.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ func getPackageSpecFromParams(cmd *cobra.Command, config *core.Config) (*core.Pa
if err != nil {
return nil, errors.Wrap(err, "")
}
concurrency, err := cmd.Flags().GetString("concurrency")
if err != nil {
return nil, errors.Wrap(err, "")
}

var envVariables []string
if cmd.Flags().Changed("env-file") {
Expand All @@ -77,6 +81,7 @@ func getPackageSpecFromParams(cmd *cobra.Command, config *core.Config) (*core.Pa
IsDev: isDev,
IsOnly: isOnly,
DeployCommand: cmd.Use,
Concurrency: concurrency,
}

return &packageSpec, nil
Expand Down
1 change: 1 addition & 0 deletions cli/src/core/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ type PackageSpec struct {
CustomPackages []CustomPackage
ImageVersion string
TargetLauncher string
Concurrency string
}

type GeneratePackageSpec struct {
Expand Down
189 changes: 189 additions & 0 deletions instant.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,189 @@
import { describe, it, expect, beforeEach, jest } from '@jest/globals'

import {
createDependencyTree,
walkDependencyTree,
concurrentifyAction
} from './instant'

describe('createDependencyTree', () => {
it('should handle a single package with no dependencies', () => {
const allPackages = {
package1: { metadata: { id: 'package1', dependencies: [] } }
}
const chosenPackageIds = ['package1']
const expectedTree = { package1: {} }
expect(createDependencyTree(allPackages, chosenPackageIds)).toEqual(
expectedTree
)
})

it('should handle multiple packages with dependencies', () => {
const allPackages = {
package1: { metadata: { id: 'package1', dependencies: ['package2'] } },
package2: { metadata: { id: 'package2', dependencies: [] } }
}
const chosenPackageIds = ['package1']
const expectedTree = { package1: { package2: {} } }
expect(createDependencyTree(allPackages, chosenPackageIds)).toEqual(
expectedTree
)
})

it('should handle complex dependency trees', () => {
const allPackages = {
package1: {
metadata: { id: 'package1', dependencies: ['package2', 'package3'] }
},
package2: { metadata: { id: 'package2', dependencies: ['package3'] } },
package3: { metadata: { id: 'package3', dependencies: [] } }
}
const chosenPackageIds = ['package1']
const expectedTree = {
package1: { package2: { package3: {} }, package3: {} }
}
expect(createDependencyTree(allPackages, chosenPackageIds)).toEqual(
expectedTree
)
})

it('should throw an error for circular dependencies', () => {
const allPackages = {
package1: { metadata: { id: 'package1', dependencies: ['package2'] } },
package2: { metadata: { id: 'package2', dependencies: ['package1'] } }
}
const chosenPackageIds = ['package1']
expect(() => createDependencyTree(allPackages, chosenPackageIds)).toThrow(
'Circular dependency detected: package1 has already been visited.'
)
})

it('should handle invalid or missing package IDs gracefully', () => {
const allPackages = {
package1: { metadata: { id: 'package1', dependencies: [] } }
}
const chosenPackageIds = ['nonExistentPackage']
expect(() => createDependencyTree(allPackages, chosenPackageIds)).toThrow(
'Invalid package ID: nonExistentPackage'
)
})
})

describe('walkDependencyTree', () => {
const mockAction = jest.fn()

beforeEach(() => {
mockAction.mockClear()
})

it('should call action on a single node tree in pre-order', async () => {
const tree = { package1: {} }
await walkDependencyTree(tree, 'pre', mockAction)
expect(mockAction).toHaveBeenCalledTimes(1)
expect(mockAction).toHaveBeenCalledWith('package1')
})

it('should call action on a single node tree in post-order', async () => {
const tree = { package1: {} }
await walkDependencyTree(tree, 'post', mockAction)
expect(mockAction).toHaveBeenCalledTimes(1)
expect(mockAction).toHaveBeenCalledWith('package1')
})

it('should walk a complex tree in pre-order and call action correctly', async () => {
const tree = {
package1: {
package2: {},
package3: {
package4: {}
}
}
}
await walkDependencyTree(tree, 'pre', mockAction)
expect(mockAction.mock.calls).toEqual([
['package1'],
['package2'],
['package3'],
['package4']
])
})

it('should walk a complex tree in post-order and call action correctly', async () => {
const tree = {
package1: {
package2: {},
package3: {
package4: {}
}
}
}
await walkDependencyTree(tree, 'post', mockAction)
expect(mockAction.mock.calls).toEqual([
['package2'],
['package4'],
['package3'],
['package1']
])
})

it('should handle an empty tree', async () => {
const tree = {}
await walkDependencyTree(tree, 'pre', mockAction)
expect(mockAction).not.toHaveBeenCalled()
})
})

describe('concurrentifyAction', () => {
it('executes actions concurrently up to the specified limit', async () => {
const action = jest
.fn()
.mockImplementation(
(id) => new Promise((resolve) => setTimeout(resolve, 100))
)
const concurrentAction = concurrentifyAction(action, 2)

const startTime = Date.now()
await Promise.all([
concurrentAction('1'),
concurrentAction('2'),
concurrentAction('3') // This should wait until one of the first two completes
])
const endTime = Date.now()

expect(action).toHaveBeenCalledTimes(3)
// Check if the total time taken is in the expected range considering concurrency limit
expect(endTime - startTime).toBeGreaterThanOrEqual(200) // At least two batches of 100ms each
})

it('does not execute the same action for a given ID more than once', async () => {
const action = jest.fn().mockResolvedValue(undefined)
const concurrentAction = concurrentifyAction(action, 2)

await Promise.all([
concurrentAction('1'),
concurrentAction('1') // This should not cause a second execution
])

expect(action).toHaveBeenCalledTimes(1)
})

it('queues actions correctly when exceeding the concurrency limit', async () => {
let activeCount = 0
const action = jest.fn().mockImplementation(async (id) => {
activeCount++
expect(activeCount).toBeLessThanOrEqual(2) // Ensure no more than 2 active at a time
await new Promise((resolve) => setTimeout(resolve, 50))
activeCount--
})
const concurrentAction = concurrentifyAction(action, 2)

await Promise.all([
concurrentAction('1'),
concurrentAction('2'),
concurrentAction('3'),
concurrentAction('4') // These should be queued
])

expect(action).toHaveBeenCalledTimes(4)
})
})
Loading

0 comments on commit 6277c2b

Please sign in to comment.