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

✨Source Primetric: Migrate Python CDK to Low-code CDK #36508

Closed
wants to merge 6 commits into from

Conversation

btkcodedev
Copy link
Collaborator

What

Migrating Source Primetric to Low-Code CDK
Closes airbytehq/airbyte-internal-issues#7029

How

Developed using (Configuration Based Source) low-code CDK

Recommended reading order

  1. spec.yaml
  2. manifest.yaml
  3. schemas/*

Tests

Integration & Acceptance
  • Build Successfull

image

🚨 User Impact 🚨

  • Spec Breaking changes, migration to low-code

Copy link

vercel bot commented Mar 26, 2024

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

Name Status Preview Comments Updated (UTC)
airbyte-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 7, 2024 5:12pm

@octavia-squidington-iii octavia-squidington-iii added area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/primetric labels Mar 26, 2024
@btkcodedev
Copy link
Collaborator Author

  • Built using connector builder,
  • Build successfull
  • Awating review :)

@marcosmarxm
Copy link
Member

@CodiumAI-Agent /review

@CodiumAI-Agent
Copy link

PR Review

⏱️ Estimated effort to review [1-5]

2, because the migration to low-code CDK involves significant changes to the structure and implementation of the connector, but the complexity is managed by the declarative nature of the low-code CDK.

🧪 Relevant tests

No

🔍 Possible issues

Possible Bug: The OAuthAuthenticator is used across multiple streams, but there's no explicit handling or mention of token expiration and refresh logic beyond the initial setup. This could potentially lead to authentication failures if the token expires within a long-running sync.

Performance Concern: The current implementation does not specify any rate limiting handling. Given that the Primetric API has rate limits, this could lead to failed requests and incomplete data syncs.

🔒 Security concerns

No

Code feedback:
relevant fileairbyte-integrations/connectors/source-primetric/source_primetric/manifest.yaml
suggestion      

Consider implementing a mechanism to handle token expiration and refresh for the OAuthAuthenticator. This ensures that the connector can handle long-running syncs without authentication issues. [important]

relevant lineauthenticator:

relevant fileairbyte-integrations/connectors/source-primetric/source_primetric/manifest.yaml
suggestion      

Implement rate limiting handling to respect the Primetric API's rate limits. This could be achieved by introducing a RateLimiter in the request configuration to avoid hitting the API too frequently. [important]

relevant linerequester:

relevant fileairbyte-integrations/connectors/source-primetric/source_primetric/manifest.yaml
suggestion      

For better error handling, consider adding specific error handling strategies for HTTP error codes returned by the Primetric API. This can improve the robustness of the connector. [medium]

relevant linerequester:

relevant fileairbyte-integrations/connectors/source-primetric/source_primetric/manifest.yaml
suggestion      

To enhance the connector's usability, consider adding more descriptive titles and descriptions in the connection_specification to guide users through the configuration process. [medium]

relevant lineconnection_specification:


✨ Review tool usage guide:

Overview:
The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

  • When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:
/review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...
[pr_reviewer]
some_config1=...
some_config2=...

See the review usage page for a comprehensive guide on using this tool.

@alafanechere alafanechere added the low-code-migration This connector has been migrated to the low-code CDK label Apr 3, 2024
@btkcodedev
Copy link
Collaborator Author

Added latest poetry version, pytest version, poetry lock file, updated docs
Build is getting passed.
Comments awaited if any

@btkcodedev
Copy link
Collaborator Author

Continued in #36957

@btkcodedev btkcodedev closed this Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/primetric low-code-migration This connector has been migrated to the low-code CDK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants