Skip to content

adapt to new effective config api fields#10312

Draft
falahat wants to merge 1 commit intomainfrom
zip_deploy_effective_config
Draft

adapt to new effective config api fields#10312
falahat wants to merge 1 commit intomainfrom
zip_deploy_effective_config

Conversation

@falahat
Copy link
Copy Markdown
Contributor

@falahat falahat commented Apr 10, 2026

Description

Updates the Local Build API calls to use the new "Effective Config" fields. Basically, we just want to call the correct API fields now.

Scenarios Tested

  • Deploy a local build app, verify env vars, auto-init, etc all work
  • Deploy a source deploy app, verify env vars, auto-init, etc all work

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces support for locally built sources in App Hosting deployments by updating the release logic to distinguish between locally built and archive-based sources. It adds the LocallyBuiltSource interface and populates build configurations such as runCommand and environment variables. A review comment identified a potential runtime crash due to missing optional chaining when accessing buildConfig properties and suggested refactoring the code to reduce redundant lookups and improve maintainability in accordance with the repository's style guide.

Comment on lines +46 to +71
const isLocallyBuilt = !!context.backendLocalBuilds[backendId];
const source = isLocallyBuilt
? {
locallyBuilt: {
userStorageUri: context.backendStorageUris[backendId],
rootDirectory: context.backendConfigs[backendId].rootDir,
runCommand: context.backendLocalBuilds[backendId]?.buildConfig.runCommand,
env: context.backendLocalBuilds[backendId]?.buildConfig.env,
},
}
: {
archive: {
userStorageUri: context.backendStorageUris[backendId],
rootDirectory: context.backendConfigs[backendId].rootDir,
},
};

return orchestrateRollout({
projectId,
backendId,
location: context.backendLocations[backendId],
buildInput: {
config: context.backendLocalBuilds[backendId]?.buildConfig,
source: {
archive: {
userStorageUri: context.backendStorageUris[backendId],
rootDirectory: context.backendConfigs[backendId].rootDir,
locallyBuiltSource: !!context.backendLocalBuilds[backendId],
},
},
source,
},
}),
);
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The current implementation has a potential runtime crash and performs redundant lookups.

  1. Safety: If context.backendLocalBuilds[backendId] exists but its buildConfig property is undefined, accessing .runCommand or .env (lines 52-53) will throw a TypeError. Optional chaining should be used for buildConfig as well.
  2. Efficiency: The code performs multiple lookups for context.backendLocalBuilds[backendId], context.backendStorageUris[backendId], and context.backendConfigs[backendId].
  3. Maintainability: Consolidating these lookups into local variables improves readability and adheres to the repository's style guide regarding reducing complexity and handling null/undefined explicitly.
    const localBuild = context.backendLocalBuilds[backendId];
    const userStorageUri = context.backendStorageUris[backendId];
    const rootDirectory = context.backendConfigs[backendId].rootDir;

    const source = localBuild
      ? {
          locallyBuilt: {
            userStorageUri,
            rootDirectory,
            runCommand: localBuild.buildConfig?.runCommand,
            env: localBuild.buildConfig?.env,
          },
        }
      : {
          archive: {
            userStorageUri,
            rootDirectory,
          },
        };

    return orchestrateRollout({
      projectId,
      backendId,
      location: context.backendLocations[backendId],
      buildInput: {
        config: localBuild?.buildConfig,
        source,
      },
    });
References
  1. Reduce nesting as much as possible and handle edge cases early. Consider helper functions or variables to encapsulate branching logic. (link)
  2. Use strict null checks and handle undefined/null explicitly. (link)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants