Set specifications only for cloud #2456
Conversation
WalkthroughAcross multiple Svelte pages for Functions and Sites (create and various settings update views), an import of isCloud from $lib/system was added. The specification field in API payloads (create and update) is now conditionally included only when isCloud is true; otherwise it is set to undefined/omitted. This applies to flows such as deploy/manual/repository/template creation and settings updates for build, events, logging, name, permissions/scopes, resource limits, runtime, schedule, timeout, repository/connect, and runtime settings. No exported/public entity signatures were changed. Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/routes/(console)/project-[region]-[project]/functions/function-[function]/settings/updateResourceLimits.svelte (1)
24-24: Function name doesn't match its purpose.The function is named
updateLogging()but it's actually updating resource limits (as evidenced by the notification message on line 56 and the CardGrid title on line 77). This naming mismatch could cause confusion during maintenance.Consider renaming to match the actual purpose:
-async function updateLogging() { +async function updateResourceLimits() {
🧹 Nitpick comments (2)
src/routes/(console)/project-[region]-[project]/functions/function-[function]/settings/updateTimeout.svelte (1)
46-46: Specification field correctly gated for cloud environments.The conditional logic properly omits the specification field when not on cloud. The ternary expression
isCloud ? $func.specification || undefined : undefinedcould be simplified toisCloud ? $func.specification : undefinedsince the|| undefinedis redundant whenisCloudis true, but the current form maintains consistency with the pattern used across the PR.Optional simplification:
- specification: isCloud ? $func.specification || undefined : undefined + specification: isCloud ? $func.specification : undefinedsrc/routes/(console)/project-[region]-[project]/sites/site-[site]/settings/updateResourceLimits.svelte (1)
22-42: Consider renaming the function to match its purpose.The function is named
updateLoggingbut it updates resource limits (specification). While this is a pre-existing issue and not introduced by this PR, consider renaming it toupdateResourceLimitsfor clarity.The conditional specification logic is correct and consistent with the PR's objectives.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/functions/create-function/manual/+page.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/functions/create-function/repository-[repository]/+page.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/functions/create-function/template-[template]/+page.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/functions/function-[function]/settings/updateBuildCommand.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/functions/function-[function]/settings/updateEvents.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/functions/function-[function]/settings/updateLogging.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/functions/function-[function]/settings/updateName.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/functions/function-[function]/settings/updatePermissions.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/functions/function-[function]/settings/updateResourceLimits.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/functions/function-[function]/settings/updateRuntime.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/functions/function-[function]/settings/updateSchedule.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/functions/function-[function]/settings/updateScopes.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/functions/function-[function]/settings/updateTimeout.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/sites/site-[site]/settings/updateBuildSettings.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/sites/site-[site]/settings/updateLogging.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/sites/site-[site]/settings/updateName.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/sites/site-[site]/settings/updateRepository.svelte(3 hunks)src/routes/(console)/project-[region]-[project]/sites/site-[site]/settings/updateResourceLimits.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/sites/site-[site]/settings/updateRuntimeSettings.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/sites/site-[site]/settings/updateTimeout.svelte(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: e2e
🔇 Additional comments (33)
src/routes/(console)/project-[region]-[project]/functions/function-[function]/settings/updateTimeout.svelte (1)
14-14: LGTM: Import added correctly.The import of
isCloudfrom$lib/systemis clean and follows the existing import patterns in the file.src/routes/(console)/project-[region]-[project]/sites/site-[site]/settings/updateRuntimeSettings.svelte (2)
14-14: LGTM: Import added correctly.
47-47: Specification field correctly gated for cloud environments.The implementation properly uses optional chaining and conditionally includes the specification field based on the cloud environment, consistent with the PR's objective.
src/routes/(console)/project-[region]-[project]/sites/site-[site]/settings/updateBuildSettings.svelte (2)
10-10: LGTM: Import added correctly.
140-140: Specification field correctly gated for cloud environments.The implementation properly validates that only enabled specifications are sent (Lines 115-120) before conditionally including the specification field based on the cloud environment. This adds an additional layer of validation beyond just the cloud check.
src/routes/(console)/project-[region]-[project]/functions/function-[function]/settings/updateScopes.svelte (2)
14-14: LGTM: Import added correctly.
49-49: Specification field correctly gated for cloud environments.The conditional logic properly omits the specification field when not on cloud, consistent with the pattern applied across all function settings update files in this PR.
src/routes/(console)/project-[region]-[project]/functions/function-[function]/settings/updateBuildCommand.svelte (2)
11-11: LGTM: Import added correctly.
40-40: Specification field correctly gated for cloud environments.The conditional logic properly omits the specification field when not on cloud, maintaining consistency with the pattern used across the PR.
src/routes/(console)/project-[region]-[project]/sites/site-[site]/settings/updateLogging.svelte (2)
11-11: LGTM: Import added correctly.
37-37: Specification field correctly gated for cloud environments.The implementation properly uses optional chaining and conditionally includes the specification field, consistent with the PR's objective to omit specifications in non-cloud environments.
src/routes/(console)/project-[region]-[project]/functions/function-[function]/settings/updateName.svelte (2)
14-14: LGTM: Import added correctly.
46-46: Specification field correctly gated for cloud environments.The conditional logic properly omits the specification field when not on cloud, maintaining the consistent pattern applied throughout this PR.
src/routes/(console)/project-[region]-[project]/functions/function-[function]/settings/updateLogging.svelte (2)
11-11: LGTM: Import added correctly.
41-41: Verify test coverage for specification gating
I ran searches for existing tests referencing thisspecificationgating and found none; please manually confirm that both cloud and non-cloud update flows are covered and add integration tests if missing.src/routes/(console)/project-[region]-[project]/functions/function-[function]/settings/updateResourceLimits.svelte (1)
14-14: LGTM! Specification gating implemented correctly.The import and conditional logic correctly ensure that the specification field is only included in the payload when running on cloud infrastructure.
Also applies to: 47-47
src/routes/(console)/project-[region]-[project]/functions/create-function/manual/+page.svelte (1)
23-23: LGTM! Specification gating implemented correctly.The conditional logic properly ensures specification is only sent for cloud deployments, aligning with the PR objective.
Also applies to: 75-75
src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.svelte (1)
23-23: LGTM! Specification gating implemented correctly.The changes correctly implement the cloud-only specification requirement, consistent with the PR's objective.
Also applies to: 109-109
src/routes/(console)/project-[region]-[project]/sites/site-[site]/settings/updateTimeout.svelte (1)
11-11: LGTM! Specification gating implemented correctly.The conditional logic properly restricts specification to cloud deployments, using the appropriate
site?.specificationreference.Also applies to: 40-40
src/routes/(console)/project-[region]-[project]/functions/function-[function]/settings/updateSchedule.svelte (1)
14-14: LGTM! Specification gating implemented correctly.The changes correctly implement cloud-only specification handling, using the appropriate store reference
$func.specification.Also applies to: 51-51
src/routes/(console)/project-[region]-[project]/functions/function-[function]/settings/updatePermissions.svelte (1)
16-16: LGTM! Specification gating implemented correctly.The conditional logic correctly ensures specification is only included for cloud deployments, consistent with other function settings updates.
Also applies to: 51-51
src/routes/(console)/project-[region]-[project]/functions/function-[function]/settings/updateEvents.svelte (1)
18-18: LGTM! Specification gating implemented correctly.The changes properly implement the cloud-only specification requirement, maintaining consistency with other function update flows.
Also applies to: 51-51
src/routes/(console)/project-[region]-[project]/sites/site-[site]/settings/updateName.svelte (1)
11-11: Verify backend handles undefinedspecification
Ensure the backend API gracefully accepts an undefinedspecificationfor non-cloud sites to avoid errors or unintended side effects.src/routes/(console)/project-[region]-[project]/functions/create-function/template-[template]/+page.svelte (2)
34-34: LGTM!The import of
isCloudis correctly placed and will be used to conditionally gate the specification field.
152-152: Cannot confirm specification gating across all flows
Our automated search found no otherfunctions.createcalls with aspecificationfield. Manually verify thatspecificationis correctly gated byisCloudin every function‐creation code path.src/routes/(console)/project-[region]-[project]/sites/site-[site]/settings/updateResourceLimits.svelte (1)
13-13: LGTM!The import is correctly added and used to conditionally include the specification field.
src/routes/(console)/project-[region]-[project]/functions/function-[function]/settings/updateRuntime.svelte (2)
14-14: LGTM!The import is correctly added.
50-50: LGTM!The specification field is correctly gated by
isCloud, reading from$func.specification. The logic ensures that specification is only passed when in a cloud environment.src/routes/(console)/project-[region]-[project]/functions/create-function/repository-[repository]/+page.svelte (2)
25-25: LGTM!The import is correctly added.
112-112: LGTM!The specification field is correctly gated by
isCloud. The implementation is consistent with the PR's objective to pass specifications only in cloud environments.src/routes/(console)/project-[region]-[project]/sites/site-[site]/settings/updateRepository.svelte (3)
10-10: LGTM!The import is correctly added.
90-90: LGTM!The specification field in
updateConfigurationis correctly gated byisCloud, using optional chaining to safely accesssite?.specification.
137-137: Allspecificationfields are gated byisCloud. Verified across every create/update flow in both site and function settings. Run integration tests to confirm:
- Cloud: specifications are passed and respected
- Non-cloud: specifications are omitted without errors
- Existing functions/sites: updates behave as before
|
Can't we just ignore it on self-hosted and send whatever is there? That way we dont need conditions for every call there. |
What does this PR do?
Only pass specifications if on cloud
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)
Related PRs and Issues
(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)
Have you read the Contributing Guidelines on issues?
(Write your answer here.)
Summary by CodeRabbit