Skip to content
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.

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

feat: change TEST_ENVIRONMENT -> TEST_ENV and log base url for CI #1198

Merged
merged 13 commits into from
Jan 15, 2025
5 changes: 2 additions & 3 deletions .github/workflows/run_js_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,10 @@ jobs:

- name: Print config file
run: |
echo "Running test with environment: ${{ inputs.environment }}"
echo "Running test with environment: ${{ inputs.environment || 'prod' }}"

- name: run test
run: cd js && TEST_ENVIRONMENT=${{ inputs.environment }} pnpm test:coverage --max-workers 16
run: cd js && TEST_ENV=${{ inputs.environment || 'prod' }} pnpm test:coverage --max-workers 16

- name: Upload `coverage` folder to R2
if: ${{ always() }}
Expand Down Expand Up @@ -126,4 +126,3 @@ jobs:
https://pub-92e668239ab84bfd80ee07d61e9d2f40.r2.dev/html-report-${{ github.run_id }}/html-report/report.html
reactions: rocket
comment-id: ${{ steps.fc.outputs.comment-id }}

6 changes: 6 additions & 0 deletions js/src/sdk/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { Triggers } from "./models/triggers";
import { ZAuthMode } from "./types/integration";
import ComposioSDKContext from "./utils/composioContext";
import { getSDKConfig } from "./utils/config";
import { IS_DEVELOPMENT_OR_CI } from "./utils/constants";
Copy link
Collaborator

Choose a reason for hiding this comment

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

The import of IS_DEVELOPMENT_OR_CI has been moved to a better location at the top with other imports, which improves code organization. However, consider grouping related imports together (e.g., all utility imports, all type imports, etc.) for better code maintainability.

import { CEG } from "./utils/error";
import { COMPOSIO_SDK_ERROR_CODES } from "./utils/errors/src/constants";
import { isNewerVersion } from "./utils/other";
Expand Down Expand Up @@ -62,6 +63,11 @@ export class Composio {
config?.apiKey
);

if (IS_DEVELOPMENT_OR_CI) {
logger.info(
`Initializing Composio w API Key: [REDACTED] and baseURL: ${baseURLParsed}`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider using a more structured logging format for initialization. Instead of string interpolation, use an object structure:

logger.info('Composio SDK Initialization', {
  baseURL: baseURLParsed,
  apiKeyPresent: !!apiKeyParsed,
  environment: process.env.NODE_ENV
});

This would make log parsing and analysis easier in production environments.

);
}
ComposioSDKContext.apiKey = apiKeyParsed;
ComposioSDKContext.sessionId = getUUID();
ComposioSDKContext.baseURL = baseURLParsed;
Expand Down
2 changes: 1 addition & 1 deletion js/src/utils/external.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export function sendProcessReq(info: {
}) {
if (IS_DEVELOPMENT_OR_CI) {
// eslint-disable-next-line no-console
console.log(
logger.debug(
`Hitting ${info.url}[${info.method}] with ${serializeValue(info.data)}`
);
return true;
Expand Down
Loading