feat: update Python cdk-deploy

[jeastham1993]

What does this PR do?

Motivation

Testing Guidelines

Additional Notes

Types of Changes

• Bug fix
• New feature
• Breaking change
• Misc (docs, refactoring, dependency upgrade, etc.)

Check all that apply

• This PR's description is comprehensive
• This PR contains breaking changes that are documented in the description
• This PR introduces new APIs or parameters that are documented and unlikely to change in the foreseeable future
• This PR impacts documentation, and it has been updated (or a ticket has been logged)
• This PR's changes are covered by the automated tests
• This PR collects user input/sensitive content into Datadog

[Copilot]

Pull Request Overview

This PR updates the Python CDK deployment process by adding build and dependency installation steps before deployment. The change ensures that the activity service is properly built and has its dependencies installed before the CDK deployment command is executed.

• Adds make dev && make deps && commands before the CDK deploy command for the activity service
• Ensures proper build and dependency setup is performed before deployment

[Copilot]

The command chain make dev && make deps && cdk deploy will fail silently if any of the make commands fail due to the background execution (&). Consider removing the background execution or adding proper error handling to ensure deployment failures are visible.

Suggested change

	make dev && make deps && cdk deploy --require-approval=never &>../../deployment-logs/activity-service.log &
	if make dev && make deps && cdk deploy --require-approval=never &>../../deployment-logs/activity-service.log; then
	echo "Activity service deployment completed successfully."
	else
	echo "Activity service deployment failed. Check the log file at ../../deployment-logs/activity-service.log for details."
	exit 1
	fi

[claude]

Claude finished @jeastham1993's task —— View job

---

PR Review: feat: update Python cdk-deploy

Reviewing this pull request for code quality, potential issues, and best practices.

Todo List:

• Read and analyze the cdk-deploy.sh file changes
• Evaluate code quality and best practices
• Check for potential bugs and issues
• Assess performance considerations
• Review security concerns
• Analyze test coverage implications
• Provide comprehensive feedback

Analysis of Changes:
The PR modifies cdk-deploy.sh:45 to add build and dependency installation steps before CDK deployment for the activity service:

- cdk deploy --require-approval=never &>../../deployment-logs/activity-service.log &
+ make dev && make deps && cdk deploy --require-approval=never &>../../deployment-logs/activity-service.log &

Code Review Feedback

✅ Code Quality & Best Practices

• Positive: Adding make dev && make deps ensures the activity service is properly built and dependencies are installed before deployment
• Positive: Consistent with deployment best practices of ensuring build artifacts are current
• Improvement: Consider consistency - other services don't have explicit build steps. If needed for activity service, consider if other services need similar treatment

⚠️ Critical Bug: Silent Failure Risk

High Priority Issue: The command chain will fail silently due to background execution (&). If make dev or make deps fails:

• The failure won't be visible to the user
• The script will continue as if deployment succeeded
• Debugging becomes difficult since errors are buried in log files

Recommended Fix:

pushd src/activity-service
if make dev && make deps && cdk deploy --require-approval=never &>../../deployment-logs/activity-service.log; then
    echo "Activity service deployment completed successfully." &
else
    echo "Activity service deployment failed. Check ../../deployment-logs/activity-service.log" >&2
    exit 1
fi &
popd

📊 Performance Considerations

• Impact: Adding build steps increases deployment time for activity service
• Trade-off: Acceptable overhead for ensuring proper build state
• Suggestion: Consider caching build artifacts or conditional builds based on file changes

🔒 Security Concerns

• Assessment: No obvious security vulnerabilities introduced
• Note: Build commands (make dev, make deps) should be audited to ensure they don't introduce security risks

🧪 Test Coverage

• Impact: No direct test coverage implications
• Suggestion: Consider adding deployment script tests or validation checks
• Missing: No verification that make dev and make deps actually succeed

🔧 Additional Recommendations

1. Error Handling: Implement proper error detection and reporting
2. Consistency: Evaluate if other services need similar build steps
3. Logging: Consider separate log files for build vs deployment phases
4. Validation: Add checks to verify build artifacts exist before deployment

Summary

The change addresses a real need (ensuring proper build/deps before deployment) but introduces a critical error handling issue. The copilot reviewer's suggestion should be implemented to prevent silent failures.