Skip to content

Update SSRC API client #2457

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Feb 20, 2024
Merged

Update SSRC API client #2457

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a01fe56
[SSRC] Add API changes needed for SSRC
trekforever Feb 13, 2024
e48a65a
Strip whitespace and improve documentation consistency
erikeldridge Feb 13, 2024
ab077c4
[SSRC] Add client internals and tests
trekforever Feb 13, 2024
8f145c6
Update URL to match API proposal
erikeldridge Feb 13, 2024
f55d1a5
Apply suggestions from code review
erikeldridge Feb 20, 2024
429394c
Merge branch 'ssrc' into ssrc-client-internals
erikeldridge Feb 20, 2024
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Apply suggestions from code review 
Co-authored-by: jen_h <harveyjen@google.com>
  • Loading branch information
@erikeldridge @jenh
erikeldridge and jenh authored Feb 20, 2024
commit f55d1a539d53fe797ff999a2a28cbb7453d3b0fd
5 changes: 4 additions & 1 deletion src/remote-config/remote-config-api-client-internal.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@ import {
} from './remote-config-api';

// Remote Config backend constants
// Honors env param to enable URL override independent of binary change.
/**
* Allows the `FIREBASE_REMOTE_CONFIG_URL_BASE` environment
* variable to override the default API endpoint URL.
*/
const FIREBASE_REMOTE_CONFIG_URL_BASE = process.env.FIREBASE_REMOTE_CONFIG_URL_BASE || 'https://firebaseremoteconfig.googleapis.com';
Copy link
Contributor Author

@erikeldridge erikeldridge Feb 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@lahirumaramba I've found it helpful here and in the Web SDK to be able to point the SDK at a local server for testing while in development, and issue reproduction after release. I don't see other references to process.env in this SDK, though. Do you have a preferred way to reference an env var?
Copy link
Member

@lahirumaramba lahirumaramba Feb 15, 2024

Choose a reason for hiding this comment

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

Thanks! This makes sense to me. We use process.env to handle other env variables

firebase-admin-node/src/storage/storage.ts

Line 48 in 5fe4118

if (!process.env.STORAGE_EMULATOR_HOST && process.env.FIREBASE_STORAGE_EMULATOR_HOST) {

Let's make a note to document this new env var in our public docs
Copy link
Contributor Author

@erikeldridge erikeldridge Feb 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Added a row to our burndown to document this env var.
const FIREBASE_REMOTE_CONFIG_HEADERS = {
'X-Firebase-Client': `fire-admin-node/${utils.getSdkVersion()}`,
Expand Down
20 changes: 10 additions & 10 deletions src/remote-config/remote-config-api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export interface RemoteConfigCondition {
}

/**
* Interface representing a Remote Config condition in the data-plane.
* Represents a Remote Config condition in the dataplane.
* A condition targets a specific group of users. A list of these conditions make up
* part of a Remote Config template.
*/
Copy link
Member

@lahirumaramba lahirumaramba Feb 15, 2024

Choose a reason for hiding this comment

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

Let's get TW reviews on these. Thanks!
Copy link
Contributor Author

@erikeldridge erikeldridge Feb 15, 2024

Choose a reason for hiding this comment

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

Ah. Good point. I was focused on this being an internal client and overlooked that it updated the API. I've added @jenh as a reviewer.
Expand Down Expand Up @@ -156,7 +156,7 @@ export interface RemoteConfigParameterGroup {
}

/**
* Interface representing a Remote Config client template.
* Represents a Remote Config client template.
*/
export interface RemoteConfigTemplate {
/**
Expand Down Expand Up @@ -189,7 +189,7 @@ export interface RemoteConfigTemplate {
}

/**
* Interface representing the data in a Remote Config server template.
* Represents the data in a Remote Config server template.
*/
export interface RemoteConfigServerTemplateData {
/**
Expand All @@ -203,7 +203,7 @@ export interface RemoteConfigServerTemplateData {
parameters: { [key: string]: RemoteConfigParameter };

/**
* ETag of the current Remote Config template (readonly).
* Current Remote Config template ETag (read-only).
*/
readonly etag: string;

Expand All @@ -214,28 +214,28 @@ export interface RemoteConfigServerTemplateData {
}

/**
* Interface representing a stateful abstraction for a Remote Config server template.
* Represents a stateful abstraction for a Remote Config server template.
*/
export interface RemoteConfigServerTemplate {

/**
* Cached {@link RemoteConfigServerTemplateData}
* Cached {@link RemoteConfigServerTemplateData}.
*/
cache: RemoteConfigServerTemplateData;

/**
* A {@link RemoteConfigServerConfig} containing default values for Config
* A {@link RemoteConfigServerConfig} that contains default Config values.
*/
defaultConfig: RemoteConfigServerConfig;

/**
* Evaluates the current template to produce a {@link RemoteConfigServerConfig}
* Evaluates the current template to produce a {@link RemoteConfigServerConfig}.
*/
evaluate(): RemoteConfigServerConfig;

/**
* Fetches and caches the current active version of the
* {@link RemoteConfigServerTemplate} of the project.
* project's {@link RemoteConfigServerTemplate}.
*/
load(): Promise<void>;
}
Expand Down Expand Up @@ -364,6 +364,6 @@ export interface ListVersionsOptions {
}

/**
* Type representing the configuration produced by evaluating a server template.
* Represents the configuration produced by evaluating a server template.
*/
export type RemoteConfigServerConfig = { [key: string]: string | boolean | number }