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

fix: Preserve __alias__ when mapping streams with repeated schema messages #1524

Merged
merged 5 commits into from
Mar 24, 2023

Conversation

DanilJr
Copy link
Contributor

@DanilJr DanilJr commented Mar 23, 2023

It relates to this issue

Closes #1521


📚 Documentation preview 📚: https://meltano-sdk--1524.org.readthedocs.build/en/1524/

@DanilJr DanilJr changed the title Mapping with context fix: mapping with context Mar 23, 2023
@codecov
Copy link

codecov bot commented Mar 23, 2023

Codecov Report

Merging #1524 (5988c83) into main (f943fa3) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1524   +/-   ##
=======================================
  Coverage   85.50%   85.51%           
=======================================
  Files          57       57           
  Lines        4692     4693    +1     
  Branches      802      802           
=======================================
+ Hits         4012     4013    +1     
  Misses        489      489           
  Partials      191      191           
Impacted Files Coverage Δ
singer_sdk/mapper.py 85.84% <100.00%> (+0.06%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Collaborator

@edgarrmondragon edgarrmondragon left a comment

Choose a reason for hiding this comment

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

Hey @DanilJr!

Thanks for the PR. Just one question. Otherwise this looks good 🙂

singer_sdk/mapper.py Outdated Show resolved Hide resolved
@edgarrmondragon edgarrmondragon changed the title fix: mapping with context fix: Preserve __alias__ is when mapping streams with repeated schema messages Mar 24, 2023
@edgarrmondragon
Copy link
Collaborator

I would like to have automated tests but at least manual testing confirms that the patch works as intended:

1. Singer input
{"type": "SCHEMA", "stream": "example", "schema": {"properties": {"id": {"type": "integer"}}}}
{"type": "RECORD", "stream": "example", "record": {"id": 1}}
{"type": "SCHEMA", "stream": "example", "schema": {"properties": {"id": {"type": "integer"}}}}
{"type": "RECORD", "stream": "example", "record": {"id": 2}}
2. Stream config
{
  "stream_maps": {
    "example": {
      "__alias__": "example_v2"
    }
  }
}
3. Mapper output
{"type": "SCHEMA", "stream": "example_v2", "schema": {"properties": {"id": {"type": "integer"}}}, "key_properties": [], "bookmark_properties": []}
{"type": "RECORD", "stream": "example_v2", "record": {"id": 1}, "time_extracted": "2023-03-24T19:41:37.953424+00:00"}
{"type": "SCHEMA", "stream": "example_v2", "schema": {"properties": {"id": {"type": "integer"}}}, "key_properties": [], "bookmark_properties": []}
{"type": "RECORD", "stream": "example_v2", "record": {"id": 2}, "time_extracted": "2023-03-24T19:41:37.953966+00:00"}

I can also confirm that using a explicit shallow copy (dict.copy()) also works.

@edgarrmondragon edgarrmondragon changed the title fix: Preserve __alias__ is when mapping streams with repeated schema messages fix: Preserve __alias__ when mapping streams with repeated schema messages Mar 24, 2023
@edgarrmondragon edgarrmondragon merged commit 88cf898 into meltano:main Mar 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: Stream map __alias__ is lost after first schema for a stream is processed
2 participants