feat: add --skip-acceleration-wait flag to skip image acceleration po…#158
Open
mozhou52 wants to merge 1 commit into
Open
feat: add --skip-acceleration-wait flag to skip image acceleration po…#158mozhou52 wants to merge 1 commit into
mozhou52 wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdds a ChangesSkip Acceleration Wait Feature
🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
mozhou52
force-pushed
the
skip-acceleration-wait
branch
from
0190158 to
b2322fe
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
__tests__/ut/commands/deploy/impl/function_test.ts (1)
246-273: ⚡ Quick winAdd one custom-container runtime case for
skipAccelerationWait.This test verifies option forwarding, but it never exercises the acceleration-specific branch because runtime stays
nodejs12. Add a case withruntime: 'custom-container'andcustomContainerConfig.imageto validate the intended behavior path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@__tests__/ut/commands/deploy/impl/function_test.ts` around lines 246 - 273, The test currently never exercises the acceleration branch; update the test case in function_test (the 'should pass skipAccelerationWait to deployFunction' spec) to set service runtime to 'custom-container' and provide a customContainerConfig.image so the code takes the custom-container acceleration path—e.g., after creating Service(mockInputs, { ...mockOpts, skipAccelerationWait: true }) set Object.defineProperty(service, 'runtime', { value: 'custom-container', writable: true }) and set service.customContainerConfig = { image: 'test/image:latest' } (or via Object.defineProperty) before calling service.run(), then assert service.fcSdk.deployFunction was called with expect.objectContaining({ skipAccelerationWait: true }) as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/resources/fc/index.ts`:
- Around line 96-100: The skip-acceleration-wait log dereferences
config.customContainerConfig.image without ensuring customContainerConfig
exists; update the block guarded by skipAccelerationWait (the conditional using
skipAccelerationWait and logger.info) to first check that
config.customContainerConfig and config.customContainerConfig.image are defined
and only include the image in the log when present, otherwise log a fallback
message (e.g., reference to custom container image unavailable) so that invoking
skipAccelerationWait doesn't throw when customContainerConfig is unset.
---
Nitpick comments:
In `@__tests__/ut/commands/deploy/impl/function_test.ts`:
- Around line 246-273: The test currently never exercises the acceleration
branch; update the test case in function_test (the 'should pass
skipAccelerationWait to deployFunction' spec) to set service runtime to
'custom-container' and provide a customContainerConfig.image so the code takes
the custom-container acceleration path—e.g., after creating Service(mockInputs,
{ ...mockOpts, skipAccelerationWait: true }) set Object.defineProperty(service,
'runtime', { value: 'custom-container', writable: true }) and set
service.customContainerConfig = { image: 'test/image:latest' } (or via
Object.defineProperty) before calling service.run(), then assert
service.fcSdk.deployFunction was called with expect.objectContaining({
skipAccelerationWait: true }) as before.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 979af726-745c-4b91-84cb-ee1e1fd3aeba
📒 Files selected for processing (5)
__tests__/ut/commands/deploy/impl/function_test.tssrc/commands-help/deploy.tssrc/resources/fc/index.tssrc/subCommands/deploy/impl/function.tssrc/subCommands/deploy/index.ts
Comment on lines
+96
to
+100
| if (skipAccelerationWait) { | ||
| logger.info( | ||
| `Skip waiting for ${config.customContainerConfig.image} optimization. The function will be available for invocation once the image acceleration process is complete.`, | ||
| ); | ||
| return; |
There was a problem hiding this comment.
Guard customContainerConfig.image before skip-log access.
Line 98 dereferences config.customContainerConfig.image, but config-only deploy can unset customContainerConfig before this method is called (see Line 297 + Line 300). That can throw at runtime and break --skip-acceleration-wait on custom-container functions.
Proposed fix
- if (skipAccelerationWait) {
+ const image = _.get(config, 'customContainerConfig.image', 'custom-container image');
+ if (skipAccelerationWait) {
logger.info(
- `Skip waiting for ${config.customContainerConfig.image} optimization. The function will be available for invocation once the image acceleration process is complete.`,
+ `Skip waiting for ${image} optimization. The function will be available for invocation once the image acceleration process is complete.`,
);
return;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (skipAccelerationWait) { | |
| logger.info( | |
| `Skip waiting for ${config.customContainerConfig.image} optimization. The function will be available for invocation once the image acceleration process is complete.`, | |
| ); | |
| return; | |
| const image = config.customContainerConfig?.image ?? 'custom-container image'; | |
| if (skipAccelerationWait) { | |
| logger.info( | |
| `Skip waiting for ${image} optimization. The function will be available for invocation once the image acceleration process is complete.`, | |
| ); | |
| return; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/resources/fc/index.ts` around lines 96 - 100, The skip-acceleration-wait
log dereferences config.customContainerConfig.image without ensuring
customContainerConfig exists; update the block guarded by skipAccelerationWait
(the conditional using skipAccelerationWait and logger.info) to first check that
config.customContainerConfig and config.customContainerConfig.image are defined
and only include the image in the log when present, otherwise log a fallback
message (e.g., reference to custom container image unavailable) so that invoking
skipAccelerationWait doesn't throw when customContainerConfig is unset.
…lling after deploy Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
mozhou52
force-pushed
the
skip-acceleration-wait
branch
from
b2322fe to
396a27d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…lling after deploy
Summary by CodeRabbit
New Features
Documentation
Tests