fix: Increase migration job timeout #1985
Conversation
📝 WalkthroughWalkthroughThis pull request introduces a new integer parameter named Changes
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
oskogstad
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.azure/applications/web-api-migration-job/main.bicep (1)
25-27: New Timeout Parameter Added: Consider Constraint AnnotationsThe new parameter
replicaTimeOutInSecondsis introduced with a clear description. For added robustness, consider adding an annotation (e.g.,@minValue(0)) to enforce that the value is non-negative. This can help avoid configuration errors when a negative timeout is inadvertently provided..azure/modules/containerAppJob/main.bicep (1)
32-34: New Timeout Parameter in Module: Validate Input ConstraintsThe
replicaTimeOutInSecondsparameter is properly added with its description. As with the main file, consider adding a constraint (for example,@minValue(0)) to guarantee that only valid (non-negative) timeout values are used in the deployment.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/Digdir.Domain.Dialogporten.Infrastructure/Persistence/Migrations/DialogDbContextModelSnapshot.csis excluded by!**/Migrations/DialogDbContextModelSnapshot.cs
📒 Files selected for processing (6)
.azure/applications/web-api-migration-job/main.bicep(2 hunks).azure/applications/web-api-migration-job/prod.bicepparam(1 hunks).azure/applications/web-api-migration-job/staging.bicepparam(1 hunks).azure/applications/web-api-migration-job/test.bicepparam(1 hunks).azure/applications/web-api-migration-job/yt01.bicepparam(1 hunks).azure/modules/containerAppJob/main.bicep(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build / build-and-test
- GitHub Check: build-infrastructure / Build infrastructure for test
🔇 Additional comments (6)
.azure/applications/web-api-migration-job/test.bicepparam (1)
6-6: New timeout parameter looks good.The introduction of
replicaTimeOutInSecondswith a value of 36000 seconds (10 hours) aligns with the PR objective to increase the migration job timeout limit..azure/applications/web-api-migration-job/staging.bicepparam (1)
6-6: New timeout parameter looks good.The introduction of
replicaTimeOutInSecondswith a value of 36000 seconds (10 hours) aligns with the PR objective to increase the migration job timeout limit..azure/applications/web-api-migration-job/yt01.bicepparam (1)
6-6: New timeout parameter looks good.The introduction of
replicaTimeOutInSecondswith a value of 36000 seconds (10 hours) aligns with the PR objective to increase the migration job timeout limit..azure/applications/web-api-migration-job/prod.bicepparam (1)
6-6: New timeout parameter looks good.The introduction of
replicaTimeOutInSecondswith a value of 36000 seconds (10 hours) aligns with the PR objective to increase the migration job timeout limit.Consider documenting the reason for choosing this specific timeout value, perhaps in a comment or in related documentation, to provide context for future maintenance.
.azure/applications/web-api-migration-job/main.bicep (1)
69-80: Parameter Mapping in Module Deployment: Confirm ConsistencyThe new parameter is passed to the
migrationJobmodule on line 79, ensuring that the dynamic timeout value is applied. Please verify that the target module correctly declares and expects an integer parameter namedreplicaTimeOutInSeconds—this will ensure end-to-end consistency..azure/modules/containerAppJob/main.bicep (1)
68-71: Replacing Hardcoded Timeout with Configurable ParameterReplacing the previously hardcoded value with the newly introduced parameter (
replicaTimeOutInSeconds) on line 70 is a good move. Ensure that all dependent logic within the job configuration interprets this value correctly.
🤖 I have created a release *beep* *boop* --- ## [1.57.1](v1.57.0...v1.57.1) (2025-02-28) ### Bug Fixes * Add missing timeout parameter for sync jobs ([#1987](#1987)) ([265bad0](265bad0)) * Increase migration job timeout ([#1985](#1985)) ([393f151](393f151)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Description
Related Issue(s)
Verification
Documentation
docs-directory, Altinnpedia or a separate linked PR in altinn-studio-docs., if applicable)