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.

Sign up for GitHub

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

Jump to bottom

🎉 Source Mailchimp: Get Member activities #3415

Merged
merged 19 commits into from
May 27, 2021
Merged

🎉 Source Mailchimp: Get Member activities #3415

merged 19 commits into from
May 27, 2021

Conversation

vovavovavovavova
Copy link
Contributor

@vovavovavovavova vovavovavovavova commented May 14, 2021 •
edited by sync-by-unito bot
Loading

What

Gets Mailchimp email-activities #1693

How

Email activities strictly belong to campaign with certain campaign_id. During looping over campaigns connector gathers their ids and then gets an email activity based on provided campaign ids.

Pre-merge Checklist

  • Run integration tests
  • Publish Docker images

Recommended reading order

  1. test.java
  2. component.ts
  3. the rest

┆Issue is synchronized with this Asana task by Unito

@auto-assign auto-assign bot requested review from davinchia and jrhizor May 14, 2021 13:09
@vovavovavovavova
Copy link
Contributor Author

vovavovavovavova commented May 14, 2021 •
edited by github-actions bot
Loading

/test connector=source-mailchimp

🕑 source-mailchimp https://github.com/airbytehq/airbyte/actions/runs/842255796
✅ source-mailchimp https://github.com/airbytehq/airbyte/actions/runs/842255796

sherifnada
sherifnada suggested changes May 14, 2021
View reviewed changes
Copy link
Contributor

@sherifnada sherifnada left a comment

Choose a reason for hiding this comment

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

few comments:

  1. could you use the new CDK package by following the pattern in this PR: Migrate CDK Stripe source to PyPi and new structure #3302
  2. Could you add a changelog.md describing the changes in this PR
  3. we need to add cursor fields

airbyte-integrations/connectors/source-mailchimp/source_mailchimp/schemas/Email_activity.json Outdated Show resolved Hide resolved
@vovavovavovavova
Copy link
Contributor Author

vovavovavovavova commented May 18, 2021 •
edited by github-actions bot
Loading

/test connector=source-mailchimp

🕑 source-mailchimp https://github.com/airbytehq/airbyte/actions/runs/854810650
❌ source-mailchimp https://github.com/airbytehq/airbyte/actions/runs/854810650

keu
keu requested changes May 21, 2021
View reviewed changes
airbyte-integrations/connectors/source-mailchimp/source_mailchimp/source.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-mailchimp/source_mailchimp/source.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-mailchimp/source_mailchimp/streams.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-mailchimp/source_mailchimp/streams.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-mailchimp/source_mailchimp/streams.py
name = "Email_activity"
data_field = "emails"

def stream_slices(self, **kwargs):
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

airbyte-integrations/connectors/source-mailchimp/source_mailchimp/schemas/Email_activity.json Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-mailchimp/source_mailchimp/streams.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-mailchimp/source_mailchimp/streams.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-mailchimp/source_mailchimp/streams.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-mailchimp/source_mailchimp/streams.py Outdated Show resolved Hide resolved
keu
keu requested changes May 24, 2021
View reviewed changes
Copy link
Contributor

@keu keu left a comment

Choose a reason for hiding this comment

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

just small item and please also double check other schemas for issues similar to what we had in email_activity

airbyte-integrations/connectors/source-mailchimp/source_mailchimp/schemas/email_activity.json Outdated Show resolved Hide resolved
keu
keu approved these changes May 24, 2021
View reviewed changes
Copy link
Contributor

@keu keu left a comment

Choose a reason for hiding this comment

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

please update configured catalog, I still can see meta fields there

@vovavovavovavova
Copy link
Contributor Author

vovavovavovavova commented May 24, 2021 •
edited by github-actions bot
Loading

/test connector=source-mailchimp

🕑 source-mailchimp https://github.com/airbytehq/airbyte/actions/runs/870729233
❌ source-mailchimp https://github.com/airbytehq/airbyte/actions/runs/870729233

@vovavovavovavova
Copy link
Contributor Author

vovavovavovavova commented May 24, 2021 •
edited by github-actions bot
Loading

/test connector=source-mailchimp

🕑 source-mailchimp https://github.com/airbytehq/airbyte/actions/runs/871113844
✅ source-mailchimp https://github.com/airbytehq/airbyte/actions/runs/871113844

sherifnada
sherifnada suggested changes May 25, 2021
View reviewed changes
airbyte-integrations/connectors/source-mailchimp/CHANGELOG.md Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-mailchimp/acceptance-test-docker.sh Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-mailchimp/source_mailchimp/streams.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-mailchimp/acceptance-test-config.yml
- config_path: "secrets/config.json"
configured_catalog_path: "integration_tests/configured_catalog.json"
validate_output_from_all_streams: yes
# incremental:
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this commented out?

Copy link
Contributor Author

@vovavovavovavova vovavovavovavova May 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

incremental in acceptance tests is currently not supported, so it is disabled in tests.
Reason: test is supposed to accept only {cursor_field: value} as state, when I need another construction.

{
  "<campaign_id>": {
  cursor_field: "<your_datetime>"
}

Copy link
Contributor

Choose a reason for hiding this comment

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

@vovavovavovavova you should add a comment here explaining this,
ideally, create a ticket for standard acceptance tests and mention it here.

@vovavovavovavova
Copy link
Contributor Author

vovavovavovavova commented May 25, 2021 •
edited by github-actions bot
Loading

/test connector=source-mailchimp

🕑 source-mailchimp https://github.com/airbytehq/airbyte/actions/runs/874211145
✅ source-mailchimp https://github.com/airbytehq/airbyte/actions/runs/874211145

@keu keu linked an issue May 26, 2021 that may be closed by this pull request
Source Mailchimp: Get Member activities #1693
Closed
@vovavovavovavova
Copy link
Contributor Author

vovavovavovavova commented May 27, 2021 •
edited by github-actions bot
Loading

/test connector=source-mailchimp

🕑 source-mailchimp https://github.com/airbytehq/airbyte/actions/runs/881458960
✅ source-mailchimp https://github.com/airbytehq/airbyte/actions/runs/881458960

@vovavovavovavova
Copy link
Contributor Author

vovavovavovavova commented May 27, 2021 •
edited by github-actions bot
Loading

/publish connector=connectors/source-mailchimp

🕑 connectors/source-mailchimp https://github.com/airbytehq/airbyte/actions/runs/881545979
✅ connectors/source-mailchimp https://github.com/airbytehq/airbyte/actions/runs/881545979

@vovavovavovavova vovavovavovavova merged commit b8e050e into master May 27, 2021
@vovavovavovavova vovavovavovavova deleted the valdemar/#1693_mailchimp_email_activities branch May 27, 2021 10:37
@avaidyanatha avaidyanatha changed the title Source Mailchimp: Get Member activities 🎉 Source Mailchimp: Get Member activities Jun 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@keu keu keu approved these changes

@sherifnada sherifnada sherifnada approved these changes

@po3na4skld po3na4skld po3na4skld approved these changes

@Saiful79NZ Saiful79NZ Saiful79NZ approved these changes

@htrueman htrueman Awaiting requested review from htrueman

@grebessi grebessi Awaiting requested review from grebessi

@Zirochkaa Zirochkaa Awaiting requested review from Zirochkaa

@vitaliizazmic vitaliizazmic Awaiting requested review from vitaliizazmic

@davinchia davinchia Awaiting requested review from davinchia

@jrhizor jrhizor Awaiting requested review from jrhizor

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Source Mailchimp: Get Member activities
5 participants
@vovavovavovavova @keu @sherifnada @po3na4skld @Saiful79NZ