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 Amplitude: enable event stream time interval selection #21022
🎉 Source Amplitude: enable event stream time interval selection #21022
Conversation
b86f8ec
to
66b1007
Compare
Issue was linked to Harvestr Discovery: Source Amplitude: Enable events stream |
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.
Thanks for the contribution!
Some things that will need to be added:
- A changelog entry
- Bump the version of the connector
This is probably a minor semver change, as this new config option is optional
airbyte-integrations/connectors/source-amplitude/source_amplitude/api.py
Outdated
Show resolved
Hide resolved
hey @mariamthiam thanks for the contribution? Can you address the comments above by adding a change log entry and bumping the version in the dockerfile of the connector? Let me know if you need help with this or if you have any questions! |
f7f4f34
to
27f7679
Compare
@evantahler and @sajarin it's done ✅ |
/test connector=connectors/source-amplitute
Build FailedTest summary info:
|
airbyte-integrations/connectors/source-amplitude/source_amplitude/api.py
Outdated
Show resolved
Hide resolved
The {
"api_key": "xxx",
"secret_key": "yyy",
"start_date": "2023-02-01T00:00:00Z",
"data_region": "Standard Server"
} |
4f8167a
to
12294e2
Compare
def __init__(self, data_region: str, **kwargs): | ||
def __init__(self, data_region: str, event_time_interval: dict = None, **kwargs): | ||
if event_time_interval is None: | ||
event_time_interval = {"size_unit": "days", "size": 1} |
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.
@evantahler default value is set here
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.
👍 lets try the test again!
/test connector=connectors/source-amplitude
Build FailedTest summary info:
|
f378626
to
8e4de5f
Compare
Sorry this should be the last fix can you rerun the tests @evantahler ? |
/test connector=connectors/source-amplitude
Build PassedTest summary info:
|
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.
🥳 I want to ask one person for a review, and then we can merge this in!
@bazarnov I pinged you for a PR review, since you recently worked on this connector. I believe that this PR looks good, but I wanted your 👀 on it |
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.
Looks good. LGTM!
/publish connector=connectors/source-amplitude
if you have connectors that successfully published but failed definition generation, follow step 4 here |
Thank you @mariamthiam! I'm getting this published and merged in now. |
What
Enable the possibility to select events stream time interval to resolve issue #21010
How
Add new source parameter
event_time_interval
to make time interval selectionable for event streamRecommended reading order
x.java
y.python
🚨 User Impact 🚨
Are there any breaking changes? What is the end result perceived by the user? If yes, please merge this PR with the 🚨🚨 emoji so changelog authors can further highlight this if needed.
Pre-merge Checklist
Expand the relevant checklist and delete the others.
Updating a connector
Community member or Airbyter
airbyte_secret
./gradlew :airbyte-integrations:connectors:<name>:integrationTest
.README.md
bootstrap.md
. See description and examplesdocs/integrations/<source or destination>/<name>.md
including changelog. See changelog exampleAirbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.
/test connector=connectors/<name>
command is passing/publish
command described hereTests
Unit
Put your unit tests output here.
Integration
Put your integration tests output here.
Acceptance
Put your acceptance tests output here.