Skip to content

Commit b9d4e4c

Browse files
committed
Address PR review feedback for container support
- Add port comments to Dockerfile EXPOSE directive - Fix container runtime null check (info.runtime !== null) - Use path.join() in getDockerfilePath for cross-platform support - Use CONTAINER_RUNTIMES constant instead of hardcoded array - Change dynamic import to static import in container-dev-server - Check ports sequentially to avoid bind race conditions
1 parent 8dac9cb commit b9d4e4c

5 files changed

Lines changed: 12 additions & 7 deletions

File tree

src/assets/container/python/Dockerfile

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@ USER bedrock_agentcore
1616

1717
COPY . .
1818

19+
# 8080: AgentCore runtime endpoint
20+
# 8000: Local dev server (uvicorn)
21+
# 9000: OpenTelemetry collector
1922
EXPOSE 8080 8000 9000
2023

2124
CMD ["opentelemetry-instrument", "python", "-m", "{{entrypoint}}"]

src/cli/external-requirements/checks.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,8 @@ export async function checkDependencyVersions(projectSpec: AgentCoreProjectSpec)
135135
let containerRuntimeAvailable = true;
136136
if (requiresContainerRuntime(projectSpec)) {
137137
const info = await detectContainerRuntime();
138-
containerRuntimeAvailable = info !== null;
139-
if (!info) {
138+
containerRuntimeAvailable = info.runtime !== null;
139+
if (!info.runtime) {
140140
// This is a warning, not an error - deploy still works via CodeBuild
141141
// We don't add to errors[] since it's not blocking
142142
}

src/cli/operations/dev/container-dev-server.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { CONTAINER_INTERNAL_PORT, DOCKERFILE_NAME } from '../../../lib';
2+
import { detectContainerRuntime, getStartHint } from '../../external-requirements/detect';
23
import { DevServer, type LogLevel, type SpawnConfig } from './dev-server';
34
import { convertEntrypointToModule } from './utils';
45
import { spawnSync } from 'child_process';
@@ -32,7 +33,6 @@ export class ContainerDevServer extends DevServer {
3233
const { onLog } = this.options.callbacks;
3334

3435
// 1. Detect container runtime
35-
const { detectContainerRuntime, getStartHint } = await import('../../external-requirements/detect');
3636
const { runtime, notReadyRuntimes } = await detectContainerRuntime();
3737
if (!runtime) {
3838
if (notReadyRuntimes.length > 0) {

src/lib/constants.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { join } from 'path';
2+
13
// Re-export all schema constants from schema
24
export * from '../schema';
35

@@ -60,5 +62,5 @@ export const START_HINTS: Record<ContainerRuntime, string> = {
6062
* Get the Dockerfile path for a given code location.
6163
*/
6264
export function getDockerfilePath(codeLocation: string): string {
63-
return `${codeLocation}/${DOCKERFILE_NAME}`;
65+
return join(codeLocation, DOCKERFILE_NAME);
6466
}

src/lib/packaging/container.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { AgentEnvSpec } from '../../schema';
2-
import { DOCKERFILE_NAME, ONE_GB } from '../constants';
2+
import { CONTAINER_RUNTIMES, DOCKERFILE_NAME, ONE_GB } from '../constants';
33
import { PackagingError } from './errors';
44
import { resolveCodeLocation } from './helpers';
55
import type { ArtifactResult, PackageOptions, RuntimePackager } from './types/packaging';
@@ -9,10 +9,10 @@ import { join } from 'path';
99

1010
/**
1111
* Detect container runtime synchronously.
12-
* Checks docker, podman, finch in order; returns the first available binary name.
12+
* Checks runtimes in CONTAINER_RUNTIMES order; returns the first available binary name.
1313
*/
1414
function detectContainerRuntimeSync(): string | null {
15-
for (const runtime of ['docker', 'finch', 'podman']) {
15+
for (const runtime of CONTAINER_RUNTIMES) {
1616
const result = spawnSync('which', [runtime], { stdio: 'pipe' });
1717
if (result.status === 0) {
1818
const versionResult = spawnSync(runtime, ['--version'], { stdio: 'pipe' });

0 commit comments

Comments
 (0)