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

commcare connector changes #45652

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

Ishankoradia
Copy link

@Ishankoradia Ishankoradia commented Sep 18, 2024

What

Ability to exclude form fields while pulling data

How

Review guide

User Impact

An extra field in the source configuration while adding the connector

Can this PR be safely reverted and rolled back?

  • YES 💚
  • NO ❌

Copy link

vercel bot commented Sep 18, 2024

Deployment failed with the following error:

The provided GitHub repository does not contain the requested branch or commit reference. Please ensure the repository is not empty.

Copy link

vercel bot commented Sep 18, 2024

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

1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
airbyte-docs ⬜️ Ignored (Inspect) Visit Preview Sep 19, 2024 2:30am

Copy link
Contributor

@natikgadzhi natikgadzhi left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution!

The code looks reasonable. I'll kick-off the CI, and I wonder if we could move this to be a low-code connector and just add field transformations to drop the columns the user doesn't need instead of having custom python code. But, /shrug

airbyte-integrations/connectors/source-commcare/Dockerfile Outdated Show resolved Hide resolved
@Ishankoradia
Copy link
Author

@natikgadzhi can you let me know if there are any other changes required ?
Also I am not sure why the CI checks are failing. Logs dont tell much.

@natikgadzhi
Copy link
Contributor

natikgadzhi commented Sep 26, 2024

/format-fix

Format-fix job started... Check job output.

❌ Job failed.

Copy link

vercel bot commented Sep 26, 2024

@Ishankoradia is attempting to deploy a commit to the Airbyte Growth Team on Vercel.

A member of the Team first needs to authorize it.

@Ishankoradia
Copy link
Author

@natikgadzhi fixed the format issues.

@natikgadzhi
Copy link
Contributor

@Ishankoradia our CI can't push changes back to your branch ;( I think it's a setting on the PR to allow admins to make changes to it. Please toggle it, I'll run /format-fix after and get this merged.

@Ishankoradia
Copy link
Author

@natikgadzhi where is this setting that I am suppose to toggle ?

Signed-off-by: Ishankoradia <[email protected]>
Copy link
Contributor

@natikgadzhi natikgadzhi left a comment

Choose a reason for hiding this comment

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

You'd need to bump the version of the connector:

  1. pyproject.toml
  2. metadata.yaml (dockerImageTag)
  3. docs/integrations/sources/commcare.md

@@ -1,8 +1,7 @@
#
# Copyright (c) 2023 Airbyte, Inc., all rights reserved.
# Copyright (c) 2022 Airbyte, Inc., all rights reserved.
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmmmmmm that's not all that relevant, but we probably don't want to change this back.

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like your forked branch was outdated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants