[source-amplitude] - Migrate to manifest-only #51601
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
octavia-squidington-iii
added
area/connectors
octavia-squidington-iii
added
the
area/documentation
| class AverageSessionLengthRecordExtractor(RecordExtractor): | ||
| """ | ||
| Create records from complex response structure | ||
| Issue: https://github.com/airbytehq/airbyte/issues/23145 |
There was a problem hiding this comment.
Since the docs link to the same issue, I'd love to have longer, more descriptive docstrings that explain what the custom component does, and why do we have to use it.
I know this might not be related to the migration itself as you're just moving them over.
There was a problem hiding this comment.
Oh look ma, I'm useful!
I'm approving with a caveat that I'd like to see green CI before merging.
Left a couple of nit comments, non-blocking.
Here is why CI is not green:
FileNotFoundError: Could not find pyproject.toml in the unit_tests directory for source-amplitude.
Which means — you want to have a pyproject.toml file in unit_tests directory that is a dependency management thing specifically for unit tests. Here is the guide that @ChristoGrab made about setting it up.
| return [] | ||
|
|
||
|
|
||
| class TransformDatetimesToRFC3339(RecordTransformation): |
There was a problem hiding this comment.
Nit, potential improvement for a follow-up PR: I think this is possible to do in the Builder already, no? Do we not support date transforms in jinja?
There was a problem hiding this comment.
Per convo I'll make this a follow-up task so we can remove the custom component.
| @@ -21,7 +22,13 @@ def config(): | |||
|
|
|||
| @pytest.fixture(scope="module") | |||
| def streams(config): | |||
| return SourceAmplitude().streams(config=config) | |||
| catalog = ( | |||
There was a problem hiding this comment.
Out of curiosity, how does that work? Why is this needed? How are they usually configured?
There was a problem hiding this comment.
Updated -- accidentally pushed before completing. intergration_test.py is updated now.
|
Can you also share the regression tests that you've done? Also, can we clarify that this change can't be reverted and why in the The rest seems fine to me once we have fixed the mock server/integration tests |
What
Eventsstream to low-codeHow
ZipfileDecoder,GzipParser,JsonLineParserto decode and parse data returned by Amplitude'seventsexport API.TransformDatetimesToRFC3339component to maintain existing stream behavior of converting all date-time strings to RFC3339 format.manifest.yamlfile for every transformation, event stream datetime fields are hard-coded into this class.airbyte-ciReview guide
manifest.yamlcomponents.pyRecommended Readings
streams.pyfor now-deletedEventsstreamairbyte-cdk'sZipfileDecoder,GzipParser, andJsonLineParserUser Impact
eventsstream state format is updated with migration to low-code, but can be parsed from either format and converted to the relevant format.Can this PR be safely reverted and rolled back?