Skip to content

Require project environment in SDK #14679

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 22 commits into from
Nov 20, 2024
Merged

Require project environment in SDK #14679

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
186822e
Require project environment in SDK
bzwrk Nov 18, 2024
d34e259
Merge branch 'master' into biz/dj-2678-make-environment-mandatory-in-…
bzwrk Nov 18, 2024
647e18c
Refactor environment check into its own method
bzwrk Nov 18, 2024
d91741b
Define a new type for project environment
bzwrk Nov 19, 2024
c1781ed
Fix comment and update CHANGELOG.md
bzwrk Nov 19, 2024
40bab31
Fix linter errors
bzwrk Nov 19, 2024
7753a5b
Fix linter errors
bzwrk Nov 19, 2024
a904e53
Update package lock
bzwrk Nov 19, 2024
5efcc32
Update gitignore
bzwrk Nov 19, 2024
45b4875
Add eslint config
bzwrk Nov 19, 2024
7abedce
Apply eslint
bzwrk Nov 19, 2024
8de3f44
Apply linter suggestions
bzwrk Nov 19, 2024
16704ee
Merge branch 'master' into biz/dj-2678-make-environment-mandatory-in-…
bzwrk Nov 19, 2024
9d5e709
Fix markdown syntax errors
bzwrk Nov 19, 2024
d0362d0
Fix tests
bzwrk Nov 19, 2024
2070190
Merge branch 'master' into biz/dj-2678-make-environment-mandatory-in-…
bzwrk Nov 19, 2024
87fb597
Update pnpm lock
bzwrk Nov 19, 2024
5b86f27
Remove pnpm lock changes
bzwrk Nov 19, 2024
041a594
Merge branch 'master' into biz/dj-2678-make-environment-mandatory-in-…
bzwrk Nov 20, 2024
a171866
Remove eslint configs
bzwrk Nov 20, 2024
96fc56b
Undo eslint change
bzwrk Nov 20, 2024
044daed
Revert reversion of eslint changes
bzwrk Nov 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/sdk/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@pipedream/sdk",
"version": "1.0.4",
"version": "1.0.5",
"description": "Pipedream SDK",
"main": "dist/server/index.js",
"module": "dist/server/index.js",
Expand Down
17 changes: 15 additions & 2 deletions packages/sdk/src/server/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,12 @@
clientSecret: string;
};

/**
* The environment in which the server client is running (e.g., "production",
* "development").
*/
export type ProjectEnvironment = "development" | "production"

Check failure on line 23 in packages/sdk/src/server/index.ts

View workflow job for this annotation

GitHub Actions / Lint Code Base 

Missing semicolon

/**
* Options for creating a server-side client.
* This is used to configure the BackendClient instance.
Expand All @@ -25,7 +31,7 @@
* The environment in which the server client is running (e.g., "production",
* "development").
*/
environment?: string;
environment?: ProjectEnvironment;

/**
* The credentials to use for authentication against the Pipedream API.
Expand Down Expand Up @@ -353,7 +359,8 @@
* @param opts - The options for configuring the server client.
*/
constructor(opts: BackendClientOpts) {
this.environment = opts.environment ?? "production";
this.ensureValidEnvironment(opts.environment)

Check failure on line 362 in packages/sdk/src/server/index.ts

View workflow job for this annotation

GitHub Actions / Lint Code Base 

Missing semicolon
this.environment = opts.environment!!

Check failure on line 363 in packages/sdk/src/server/index.ts

View workflow job for this annotation

GitHub Actions / Lint Code Base 

Forbidden extra non-null assertion

Check failure on line 363 in packages/sdk/src/server/index.ts

View workflow job for this annotation

GitHub Actions / Lint Code Base 

Missing semicolon
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove redundant non-null assertion

The double non-null assertion (!!) is unnecessary since ensureValidEnvironment already validates the environment parameter.

-    this.ensureValidEnvironment(opts.environment)
-    this.environment = opts.environment!!
+    this.ensureValidEnvironment(opts.environment);
+    this.environment = opts.environment;
📝 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
this.ensureValidEnvironment(opts.environment)
this.environment = opts.environment!!
this.ensureValidEnvironment(opts.environment);
this.environment = opts.environment;
🧰 Tools
🪛 GitHub Check: Lint Code Base

[failure] 361-361:
Missing semicolon

[failure] 362-362:
Forbidden extra non-null assertion

[failure] 362-362:
Missing semicolon

🪛 Biome

[error] 362-362: Forbidden extra non-null assertion.

Safe fix: Remove extra non-null assertion.

(lint/suspicious/noExtraNonNullAssertion)

🪛 eslint

[error] 361-362: Missing semicolon.

(@typescript-eslint/semi)

[error] 362-362: Forbidden extra non-null assertion.

(@typescript-eslint/no-extra-non-null-assertion)


this.projectId = opts.projectId;
if (!this.projectId) {
Expand All @@ -370,6 +377,12 @@
this.oauthClient = this.newOauthClient(opts.credentials, this.baseApiUrl);
}

private ensureValidEnvironment(environment?: string) {
if (!environment || !["development", "production"].includes(environment)) {

Check failure on line 381 in packages/sdk/src/server/index.ts

View workflow job for this annotation

GitHub Actions / Lint Code Base 

A linebreak is required after '['

Check failure on line 381 in packages/sdk/src/server/index.ts

View workflow job for this annotation

GitHub Actions / Lint Code Base 

There should be a linebreak after this element

Check failure on line 381 in packages/sdk/src/server/index.ts

View workflow job for this annotation

GitHub Actions / Lint Code Base 

A linebreak is required before ']'
throw new Error("Project environment is required. Supported environments are development and production.");
}
}

private newOauthClient(
{
clientId,
Expand Down
Loading