Skip to content

Inject plainly sdk instead of using a global instance#15

Merged
danixeee merged 3 commits intomainfrom
refactor-api-client
Sep 26, 2025
Merged

Inject plainly sdk instead of using a global instance#15
danixeee merged 3 commits intomainfrom
refactor-api-client

Conversation

@danixeee
Copy link
Copy Markdown
Contributor

@danixeee danixeee commented Sep 25, 2025

  • removed the global api client that was reading api key from env
  • base for other transports (potentially more refactor will be needed for server file)
  • improved plainly sdk
  • removed api check on startup

@danixeee danixeee requested a review from ivansenic September 25, 2025 15:24
Copy link
Copy Markdown
Contributor

@ivansenic ivansenic left a comment

Choose a reason for hiding this comment

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

Very good, not sure about this one thing?

Comment thread src/index.ts

const server = new PlainlyMcpServer();
const apiClient = createApiClient({
baseUrl: env.PLAINLY_API_URL,
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.

wait is there default for this? this should not be given

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes, in env.ts:

mcp-server/src/env.ts

Lines 8 to 11 in 067dd00

PLAINLY_APP_URL:
process.env.PLAINLY_APP_URL || "https://app.plainlyvideos.com",
PLAINLY_API_URL:
process.env.PLAINLY_API_URL || "https://api.plainlyvideos.com",

e.g. in Smithery you can set env variables for your server, but api key is set through user configuration. So I think it's fine for defaults to be in env.ts

@danixeee danixeee requested a review from ivansenic September 26, 2025 09:16
@danixeee danixeee merged commit 6cfac93 into main Sep 26, 2025
1 check passed
@danixeee danixeee deleted the refactor-api-client branch September 26, 2025 10:45
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