Move 'location' from firebase.json to dataconnect.yaml (#7373)

* Remove flags in preparation for next DataConnect emulator release * Moving location to dataconnect.yaml * Update JSON schema * Cleaning up last bits of location * format * Add warning for location in firebase.json * PR fixes and VSCode fixes
firebase · Jun 25, 2024 · f3156d1 · f3156d1
1 parent 1f579c8
commit f3156d1
Show file tree

Hide file tree

Showing 16 changed files with 49 additions and 43 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -0,0 +1 @@
+- Moved `dataconnect.location` key in `firebase.json` to `dataconnect.yaml`.
diff --git a/firebase-vscode/src/data-connect/config.ts b/firebase-vscode/src/data-connect/config.ts
@@ -113,7 +113,7 @@ export async function _readDataConnectConfigs(
           absoluteLocation,
           dataConnectYaml,
           resolvedConnectors,
-          dataConnect.location,
+          dataConnectYaml.location,
         );
       }),
     );

diff --git a/schema/dataconnect-yaml.json b/schema/dataconnect-yaml.json
@@ -58,6 +58,10 @@
       "type": "string",
       "description": "The ID of the Firebase Data Connect service."
     },
+    "location": {
+      "type": "string",
+      "description": "The region of the Firebase Data Connect service."
+    },
     "connectorDirs": {
       "type": "array",
       "items": {

diff --git a/schema/firebase-config.json b/schema/firebase-config.json
@@ -276,9 +276,6 @@
                 {
                     "additionalProperties": false,
                     "properties": {
-                        "location": {
-                            "type": "string"
-                        },
                         "postdeploy": {
                             "anyOf": [
                                 {
@@ -310,7 +307,6 @@
                         }
                     },
                     "required": [
-                        "location",
                         "source"
                     ],
                     "type": "object"
@@ -319,9 +315,6 @@
                     "items": {
                         "additionalProperties": false,
                         "properties": {
-                            "location": {
-                                "type": "string"
-                            },
                             "postdeploy": {
                                 "anyOf": [
                                     {
@@ -353,7 +346,6 @@
                             }
                         },
                         "required": [
-                            "location",
                             "source"
                         ],
                         "type": "object"

diff --git a/src/commands/dataconnect-sdk-generate.ts b/src/commands/dataconnect-sdk-generate.ts
@@ -20,7 +20,7 @@ export const command = new Command("dataconnect:sdk:generate")
         const cwd = options.cwd || process.cwd();
         configDir = path.resolve(path.join(cwd), configDir);
       }
-      const serviceInfo = await load(projectId, service.location, configDir);
+      const serviceInfo = await load(projectId, configDir);
       const hasGeneratables = serviceInfo.connectorInfo.some((c) => {
         return (
           c.connectorYaml.generate?.javascriptSdk ||
@@ -38,7 +38,6 @@ export const command = new Command("dataconnect:sdk:generate")
       for (const conn of serviceInfo.connectorInfo) {
         const output = await DataConnectEmulator.generate({
           configDir,
-          locationId: service.location,
           connectorId: conn.connectorYaml.connectorId,
         });
         logger.info(output);

diff --git a/src/config.ts b/src/config.ts
@@ -91,6 +91,17 @@ export class Config {
         }
       }
     }
+
+    if (
+      this._src.dataconnect?.location ||
+      (Array.isArray(this._src.dataconnect) && this._src.dataconnect.some((c: any) => c?.location))
+    ) {
+      utils.logLabeledWarning(
+        "dataconnect",
+        "'location' has been moved from 'firebase.json' to 'dataconnect.yaml'. " +
+          "Please remove 'dataconnect.location' from 'firebase.json' and add it as top level field to 'dataconnect.yaml' instead ",
+      );
+    }
   }
 
   materialize(target: string) {

diff --git a/src/dataconnect/fileUtils.ts b/src/dataconnect/fileUtils.ts
@@ -13,14 +13,11 @@ export function readFirebaseJson(config: Config): DataConnectMultiple {
     return [];
   }
   const validator = (cfg: any) => {
-    if (!cfg["source"] && !cfg["location"]) {
-      throw new FirebaseError(
-        "Invalid firebase.json: DataConnect requires `source` and `location`",
-      );
+    if (!cfg["source"]) {
+      throw new FirebaseError("Invalid firebase.json: DataConnect requires `source`");
     }
     return {
       source: cfg["source"],
-      location: cfg["location"],
     };
   };
   const configs = config.get("dataconnect");
@@ -30,7 +27,7 @@ export function readFirebaseJson(config: Config): DataConnectMultiple {
     return configs.map(validator);
   } else {
     throw new FirebaseError(
-      "Invalid firebase.json: dataconnect should be of the form { source: string, location: string }",
+      "Invalid firebase.json: dataconnect should be of the form { source: string }",
     );
   }
 }
@@ -42,7 +39,10 @@ export async function readDataConnectYaml(sourceDirectory: string): Promise<Data
 }
 
 function validateDataConnectYaml(unvalidated: any): DataConnectYaml {
-  // TODO: Add validation
+  // TODO: Use json schema for validation here!
+  if (!unvalidated["location"]) {
+    throw new FirebaseError("Missing required field 'location' in dataconnect.yaml");
+  }
   return unvalidated as DataConnectYaml;
 }
 
@@ -89,14 +89,14 @@ export async function pickService(
   if (serviceCfgs.length === 0) {
     throw new FirebaseError("No Data Connect services found in firebase.json.");
   } else if (serviceCfgs.length === 1) {
-    serviceInfo = await load(projectId, serviceCfgs[0].location, serviceCfgs[0].source);
+    serviceInfo = await load(projectId, serviceCfgs[0].source);
   } else {
     if (!serviceId) {
       throw new FirebaseError(
         "Multiple Data Connect services found in firebase.json. Please specify a service ID to use.",
       );
     }
-    const infos = await Promise.all(serviceCfgs.map((c) => load(projectId, c.location, c.source)));
+    const infos = await Promise.all(serviceCfgs.map((c) => load(projectId, c.source)));
     // TODO: handle cases where there are services with the same ID in 2 locations.
     const maybe = infos.find((i) => i.dataConnectYaml.serviceId === serviceId);
     if (!maybe) {

diff --git a/src/dataconnect/load.ts b/src/dataconnect/load.ts
@@ -5,13 +5,9 @@ import { ServiceInfo, toDatasource, SCHEMA_ID } from "./types";
 /**
  * loads schemas and connectors from  {sourceDirectory}/dataconnect.yaml
  */
-export async function load(
-  projectId: string,
-  locationId: string,
-  sourceDirectory: string,
-): Promise<ServiceInfo> {
+export async function load(projectId: string, sourceDirectory: string): Promise<ServiceInfo> {
   const dataConnectYaml = await fileUtils.readDataConnectYaml(sourceDirectory);
-  const serviceName = `projects/${projectId}/locations/${locationId}/services/${dataConnectYaml.serviceId}`;
+  const serviceName = `projects/${projectId}/locations/${dataConnectYaml.location}/services/${dataConnectYaml.serviceId}`;
   const schemaDir = path.join(sourceDirectory, dataConnectYaml.schema.source);
   const schemaGQLs = await fileUtils.readGQLFiles(schemaDir);
   const connectorInfo = await Promise.all(
@@ -36,7 +32,11 @@ export async function load(
     sourceDirectory,
     schema: {
       name: `${serviceName}/schemas/${SCHEMA_ID}`,
-      primaryDatasource: toDatasource(projectId, locationId, dataConnectYaml.schema.datasource),
+      primaryDatasource: toDatasource(
+        projectId,
+        dataConnectYaml.location,
+        dataConnectYaml.schema.datasource,
+      ),
       source: {
         files: schemaGQLs,
       },

diff --git a/src/dataconnect/types.ts b/src/dataconnect/types.ts
@@ -102,6 +102,7 @@ export interface DataConnectYaml {
   specVersion?: string;
   serviceId: string;
   schema: SchemaYaml;
+  location: string;
   connectorDirs: string[];
 }
 

diff --git a/src/deploy/dataconnect/prepare.ts b/src/deploy/dataconnect/prepare.ts
@@ -25,9 +25,7 @@ export default async function (context: any, options: Options): Promise<void> {
   utils.logLabeledBullet("dataconnect", `Preparing to deploy`);
   const filters = getResourceFilters(options);
   const serviceInfos = await Promise.all(
-    serviceCfgs.map((c) =>
-      load(projectId, c.location, path.join(options.cwd || process.cwd(), c.source)),
-    ),
+    serviceCfgs.map((c) => load(projectId, path.join(options.cwd || process.cwd(), c.source))),
   );
   for (const si of serviceInfos) {
     si.deploymentMetadata = await build(options, si.sourceDirectory);

diff --git a/src/emulator/controller.ts b/src/emulator/controller.ts
@@ -839,7 +839,6 @@ export async function startAll(
       projectId,
       auto_download: true,
       configDir,
-      locationId: config[0].location,
       rc: options.rc,
     });
     await startEmulator(dataConnectEmulator);

diff --git a/src/emulator/dataconnectEmulator.ts b/src/emulator/dataconnectEmulator.ts
@@ -12,6 +12,7 @@ import { BuildResult, requiresVector } from "../dataconnect/types";
 import { listenSpecsToString } from "./portUtils";
 import { Client, ClientResponse } from "../apiv2";
 import { EmulatorRegistry } from "./registry";
+import { logger } from "../logger";
 import { load } from "../dataconnect/load";
 import { isVSCodeExtension } from "../utils";
 import { EventEmitter } from "events";
@@ -20,14 +21,12 @@ export interface DataConnectEmulatorArgs {
   projectId: string;
   listen: ListenSpec[];
   configDir: string;
-  locationId: string;
   auto_download?: boolean;
   rc: RC;
 }
 
 export interface DataConnectGenerateArgs {
   configDir: string;
-  locationId: string;
   connectorId: string;
 }
 
@@ -82,7 +81,6 @@ export class DataConnectEmulator implements EmulatorInstance {
         auto_download: this.args.auto_download,
         listen: listenSpecsToString(this.args.listen),
         config_dir: this.args.configDir,
-        service_location: this.args.locationId,
       });
       this.usingExistingEmulator = false;
     }
@@ -139,16 +137,22 @@ export class DataConnectEmulator implements EmulatorInstance {
       "--logtostderr",
       "-v=2",
       "generate",
-      `--service_location=${args.locationId}`,
       `--config_dir=${args.configDir}`,
       `--connector_id=${args.connectorId}`,
     ];
     const res = childProcess.spawnSync(commandInfo.binary, cmd, { encoding: "utf-8" });
+
+    logger.info(res.stderr);
     if (res.error) {
       throw new FirebaseError(`Error starting up Data Connect generate: ${res.error.message}`, {
         original: res.error,
       });
     }
+    if (res.status !== 0) {
+      throw new FirebaseError(
+        `Unable to generate your Data Connect SDKs (exit code ${res.status}): ${res.stderr}`,
+      );
+    }
     return res.stdout;
   }
 
@@ -192,7 +196,7 @@ export class DataConnectEmulator implements EmulatorInstance {
     if (!emuInfo) {
       return false;
     }
-    const serviceInfo = await load(this.args.projectId, this.args.locationId, this.args.configDir);
+    const serviceInfo = await load(this.args.projectId, this.args.configDir);
     const sameService = emuInfo.services.find(
       (s) => serviceInfo.dataConnectYaml.serviceId === s.serviceId,
     );

diff --git a/src/emulator/downloadableEmulators.ts b/src/emulator/downloadableEmulators.ts
@@ -292,7 +292,6 @@ const Commands: { [s in DownloadableEmulators]: DownloadableEmulatorCommand } =
     optionalArgs: [
       "listen",
       "config_dir",
-      "service_location",
       "disable_sdk_generation",
       "resolvers_emulator",
       "rpc_retry_count",

diff --git a/src/firebaseConfig.ts b/src/firebaseConfig.ts
@@ -241,8 +241,6 @@ export type ExtensionsConfig = Record<string, string>;
 export type DataConnectSingle = {
   // The directory containing dataconnect.yaml for this service
   source: string;
-  // The location to deploy this service to (ie 'us-central1')
-  location: string;
 } & Deployable;
 
 export type DataConnectMultiple = DataConnectSingle[];

diff --git a/src/init/features/dataconnect/index.ts b/src/init/features/dataconnect/index.ts
@@ -65,10 +65,7 @@ export async function doSetup(setup: Setup, config: Config): Promise<void> {
   const subbedDataconnectYaml = subValues(DATACONNECT_YAML_TEMPLATE, info);
   const subbedConnectorYaml = subValues(CONNECTOR_YAML_TEMPLATE, info);
 
-  if (!config.has("dataconnect")) {
-    config.set("dataconnect.source", dir);
-    config.set("dataconnect.location", info.locationId);
-  }
+  config.set("dataconnect", { source: dir });
   await config.askWriteProjectFile(join(dir, "dataconnect.yaml"), subbedDataconnectYaml);
   await config.askWriteProjectFile(join(dir, "schema", "schema.gql"), SCHEMA_TEMPLATE);
   await config.askWriteProjectFile(
@@ -108,13 +105,15 @@ function subValues(
     cloudSqlInstanceId: string;
     cloudSqlDatabase: string;
     connectorId: string;
+    locationId: string;
   },
 ): string {
   const replacements: Record<string, string> = {
     serviceId: "__serviceId__",
     cloudSqlDatabase: "__cloudSqlDatabase__",
     cloudSqlInstanceId: "__cloudSqlInstanceId__",
     connectorId: "__connectorId__",
+    locationId: "__location__",
   };
   let replaced = template;
   for (const [k, v] of Object.entries(replacementValues)) {

diff --git a/templates/init/dataconnect/dataconnect.yaml b/templates/init/dataconnect/dataconnect.yaml
@@ -1,5 +1,6 @@
 specVersion: "v1alpha"
 serviceId: "__serviceId__"
+location: "__location__"
 schema:
   source: "./schema"
   datasource:
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		- Moved `dataconnect.location` key in `firebase.json` to `dataconnect.yaml`.