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

Get all connectors to show up in oas spec #279

Merged
merged 2 commits into from
Feb 25, 2025
Merged

Get all connectors to show up in oas spec #279

merged 2 commits into from
Feb 25, 2025

Conversation

openint-bot
Copy link
Collaborator

@openint-bot openint-bot commented Feb 25, 2025

Important

This PR updates the OpenAPI specification to include all connectors by adding new schemas and updating models.

  • Behavior:
    • Ensures all connectors are included in the OpenAPI specification by updating openapi.json and openapi.types.ts.
    • Adds new schemas for connectors like aircall, airtable, apollo, beancount, brex, coda, confluence, debug, discord, finch, firebase, foreceipt, fs, github, gong, google, googledrive, greenhouse, heron, hubspot, intercom, jira, kustomer, lever, linear, lunchmoney, mercury, merge, microsoft, mongodb, moota, onebrick, outreach, pipedrive, plaid, postgres, qbo, ramp, revert, salesforce, salesloft, saltedge, slack, splitwise, spreadsheet, stripe, teller, toggl, twenty, venmo, webhook, wise, xero, yodlee, zohodesk.
  • Models:
    • Updates models.ts to use connectorSchemas for defining core.connection and core.connector_config.
  • Schemas:
    • Introduces connectorSchemas.ts to define schemas for all connectors using zod and zod-openapi.

This description was created by Ellipsis for 0cabf6f. It will automatically update as commits are pushed.

Copy link

vercel bot commented Feb 25, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
openint ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 25, 2025 3:44am

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to 0cabf6f in 2 minutes and 37 seconds

More details
  • Looked at 9369 lines of code in 5 files
  • Skipped 0 files when reviewing.
  • Skipped posting 8 drafted comments based on config settings.
1. packages-v1/api-v1/connectorSchemas.ts:20
  • Draft comment:
    Consider adding a type-check or guard when accessing defConnectors schemas. Currently, the code casts (schemas[key] as unknown as z.ZodTypeAny | undefined) and defaults to z.null() if undefined. This may hide missing schema definitions.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%
    The code already has comments explaining why the type casting approach was chosen - it's due to zod version mismatches. The fallback to z.null() appears intentional. The comment suggests an alternative approach but doesn't consider the existing constraints that led to this implementation. Type checking might make the code more defensive but could also complicate the version mismatch handling.
    I might be putting too much faith in the existing comments - perhaps the version mismatch issue could be solved another way. The suggestion for type checking could potentially catch real issues.
    While type checking could add safety, the current approach appears to be a deliberate workaround for specific technical constraints. The comment doesn't provide enough concrete value given the existing context.
    Delete the comment as it doesn't fully consider the documented technical constraints and doesn't provide clear actionable value.
2. packages-v1/api-v1/models.ts:30
  • Draft comment:
    The use of parseNonEmpty to assert non-empty array before constructing a discriminatedUnion is clever. Ensure that this behavior matches the expectation if no connectors are defined (the error thrown may be too abrupt in production).
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. packages-v1/api-v1/connectorSchemas.ts:29
  • Draft comment:
    The cast in (schemas[key] as unknown as z.ZodTypeAny | undefined) ?? z.null() is used to work around version mismatches. Consider aligning your Zod versions to remove this hack.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%
    While the comment identifies a real issue (version mismatch requiring a hack), suggesting to align versions may not be actionable. The comments in the code suggest this is a known limitation due to how zod-openapi works. Aligning versions could break other functionality. Without knowing the full dependency tree and implications, this suggestion could be more harmful than helpful.
    I might be too cautious - aligning versions could be a valid improvement. The code comments do indicate this is a known pain point that could be worth addressing.
    However, version alignment across packages is a complex architectural decision that requires broader context. The current hack, while not ideal, is documented and working.
    The comment should be deleted as it suggests a potentially risky change without sufficient context about the broader implications of version alignment.
4. packages-v1/api-v1/models.ts:33
  • Draft comment:
    The code relies on accessing .value on a Zod literal (s.shape.connector_name.value) to form the OpenAPI ref. This may be fragile if Zod internals change. Consider a helper to safely extract the literal value.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
5. connectors/connector-yodlee/YodleeClient.ts:594
  • Draft comment:
    Typo: In the comment '// TODO: Merge with YodleeError somehow... Otherwise isIntance fails :(', 'isIntance' should be corrected to 'isInstance'.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
6. packages-v1/api-v1/__generated__/openapi.types.ts:418
  • Draft comment:
    It appears that the connector name 'foreceipt' might be a typo. Please verify if it should be spelled differently (e.g. 'for receipt' or another intended term).
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50%
    The comment suggests "foreceipt" could be a typo, but there's strong evidence this is actually an intentional name: 1) It's used consistently in type definitions 2) It has proper TypeScript interfaces defined 3) It follows the same pattern as other connectors 4) It appears to be a real service name. The comment is making an incorrect assumption.
    Could this be a legitimate concern about confusing naming that should be addressed? Could there be value in making the name more clear?
    While clear naming is important, this appears to be an actual product/service name that we shouldn't second-guess. The consistency of its usage suggests it's intentional.
    Delete the comment. "foreceipt" is clearly an intentional connector name, not a typo.
7. packages-v1/api-v1/__generated__/openapi.types.ts:2164
  • Draft comment:
    The comment for the 500 response reads 'Internal server error error (500)'. Consider removing the duplicated 'error' for clarity.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
8. packages-v1/api-v1/connectorSchemas.ts:7
  • Draft comment:
    Consider replacing 'Dedupe' with 'Deduplicate' for better clarity and formality.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50%
    This is an extremely minor stylistic suggestion about wording in a comment. It doesn't affect functionality, readability is not significantly impacted as "dedupe" is common terminology, and it doesn't suggest any actual code changes. This feels like unnecessary nitpicking that doesn't add value to the code review.
    Perhaps clearer documentation is important for maintainability, and "Deduplicate" is more professional and explicit.
    "Dedupe" is a well-understood technical term, and this level of nitpicking on comment wording violates our rule about not making purely informative comments that don't require code changes.
    This comment should be deleted as it's a trivial stylistic suggestion about comment wording that doesn't impact code quality or functionality.

Workflow ID: wflow_sYVR4MuLnVVuuuS7


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link

recurseml bot commented Feb 25, 2025

😱 Found 1 issue. Time to roll up your sleeves! 😱

@pellicceama
Copy link
Collaborator

So great to have these in the spec

@pellicceama pellicceama merged commit 632ec01 into main Feb 25, 2025
4 of 5 checks passed
@pellicceama pellicceama deleted the v1-continued branch February 25, 2025 13:35
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