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 MailGun: Migrate Python CDK to Low-code CDK #29122
Conversation
Before Merging a Connector Pull RequestWow! What a great pull request you have here! 🎉 To merge this PR, ensure the following has been done/considered for each connector added or updated:
If the checklist is complete, but the CI check is failing,
|
Waiting for #28471 for merging to update start_date |
Go ahead and fix the start date directly here. |
@marcosmarxm It's Hitting with backward compatibility test errors, any suggestions? |
airbyte-integrations/connectors/source-mailgun/acceptance-test-config.yml
Show resolved
Hide resolved
…-config.yml Co-authored-by: Marcos Marx <marcosmarxm@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some comments, the connector is missing pagination.
enabled: false | ||
cloud: | ||
enabled: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why ar eyou changing this?
enabled: true | ||
oss: | ||
enabled: true | ||
releaseDate: TODO |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a date here
paginator: | ||
type: NoPagination |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add pagination as it was implemented before
airbyte/airbyte-integrations/connectors/source-mailgun/source_mailgun/source.py
Lines 42 to 49 in b78ad4e
def next_page_token(self, response: requests.Response) -> Optional[Mapping[str, HttpUrl]]: | |
""" | |
:param response: the most recent response from the API | |
:return If there is another page in the result, a mapping (e.g: dict) containing information needed to query the next page in the response. | |
If there are no more pages in the result, return None. | |
""" | |
next_page: Optional[HttpUrl] = response.json().get("paging", {}).get("next") | |
return {"url": next_page} if next_page and self._pre_parse_response(response) else None |
stream_names: | ||
- "domains" | ||
- "events" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EOF
All comments resolved |
@btkcodedev getting an error with the check command with the events stream:
|
@btkcodedev thanks for the update, config validation is now passing but no records are being fetched. I pinged you on slack for credentials so we can get this moving. |
Thanks for the contribution @btkcodedev! |
@btkcodedev @sajarin Thanks for the contribution. I'm still facing the issue with sync resulting in 0 records when I tried this today on Airbyte Cloud. Is this already resolved?
|
@padala can you make a new issue and tag me? Perhaps an issue with the record selector |
Co-authored-by: Marcos Marx <marcosmarxm@users.noreply.github.com> Co-authored-by: Sajarin <sajarindider@gmail.com>
What
Migrating Source Mailgun to Low-Code CDK
Closes #29120
How
Developed using (Configuration Based Source) low-code CDK
Recommended reading order
spec.yaml
manifest.yaml
schemas/*
Tests
Integration & Acceptance
Full Test Results🚨 User Impact 🚨
Migrate Connector: Source Mailgun
Community member or Airbyter
airbyte_secret
./gradlew :airbyte-integrations:connectors:<name>:integrationTest
.README.md
bootstrap.md
docs/integrations/<source or destination>/<name>.md
including changelog. See changelog exampledocs/integrations/README.md
airbyte-integrations/builds.md