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 Freshsales: Make Connector Compatible with Builder #38153

Merged
merged 7 commits into from
May 28, 2024

Conversation

pabloescoder
Copy link
Collaborator

What

Makes the connector compatible with the Builder UI

How

  • Use Poetry instead of requirements.txt & Dockerfile
  • Import schema to Inline Schema in manifest
  • Bump minor version
  • Make it compatible with the Builder

User Impact

Non impacting, done for maintainability

Can this PR be safely reverted and rolled back?

  • YES 💚

Copy link

vercel bot commented May 13, 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 May 28, 2024 7:09pm

@pabloescoder
Copy link
Collaborator Author

The filter streams are used as parent streams for other streams, I couldn't find a way to use those streams only internally and not expose them to the users while migrating this to the builder. If there's a way to specify that a stream needs to be used only internally but not exposed to the users, that could be great. This will be useful for streams that are to be used as parent streams for other streams but not as standalone streams. Maybe a checkbox to denote that the stream is internal only in the builder.

@@ -20,6 +20,13 @@ acceptance_tests:
- config_path: "secrets/config.json"
expect_records:
path: "integration_tests/expected_records.jsonl"
empty_streams:
- name: "sales_accounts_filter"
Copy link
Contributor

Choose a reason for hiding this comment

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

are the tests failing on master too?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The master branch does not have these filter streams exposed to the user. The filter streams are used only internally in the manifest.
However when using the builder, if I want to use the filter streams as a parent stream for another stream, I need to expose the streams. So basically the user will be able to see 3 new streams. But we don't want the user to use these streams.
Having a checkbox such as use internally only in the builder would be a nice addition where the stream will not be exposed to the user, but we could still be able to use the stream as a parent stream for another stream.

@girarda girarda merged commit 2dc3288 into master May 28, 2024
31 of 32 checks passed
@girarda girarda deleted the source-freshsales-make-builder-compatible branch May 28, 2024 22:04
xiaohansong pushed a commit that referenced this pull request May 29, 2024
Co-authored-by: Alexandre Girard <alexandre@airbyte.io>
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/freshsales
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants