Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: Make conform_record_data_types work on nested objects and arrays #887

Merged

Conversation

Jack-Burnett
Copy link
Contributor

Closes #800

conform_record_data_types is responsible for converting values into json-legal form and removing fields that are not in the catalog.

Previously conform_record_data_types only worked on primitive fields on the given record.
This PR expands it to also work on lists of primitives, nested objects, and lists of objects.

Since the options available for a list are pretty wide in jsonschema, this PR only addresses lists of uniform type (e.g. a single schema applies to the whole list, as opposed to different schemas for different indices), as this is the most common type in my experience.

When logging a list of fields it has removed, it will log them with their path, e.g. "base_obect_name.child_object.primitive_value".

@codecov
Copy link

codecov bot commented Aug 5, 2022

Codecov Report

Merging #887 (69f0b18) into main (bae2b05) will increase coverage by 0.67%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #887      +/-   ##
==========================================
+ Coverage   83.85%   84.53%   +0.67%     
==========================================
  Files          44       44              
  Lines        4151     4189      +38     
  Branches      718      725       +7     
==========================================
+ Hits         3481     3541      +60     
+ Misses        490      474      -16     
+ Partials      180      174       -6     
Impacted Files Coverage Δ
singer_sdk/helpers/_typing.py 72.30% <100.00%> (+19.51%) ⬆️
singer_sdk/streams/core.py 85.88% <100.00%> (+0.13%) ⬆️

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

@Jack-Burnett Jack-Burnett marked this pull request as draft August 5, 2022 11:11
@Jack-Burnett Jack-Burnett marked this pull request as ready for review August 5, 2022 11:34
@Jack-Burnett Jack-Burnett marked this pull request as draft August 5, 2022 11:44
@Jack-Burnett Jack-Burnett marked this pull request as ready for review August 5, 2022 13:54
@aaronsteers
Copy link
Contributor

aaronsteers commented Aug 9, 2022

@Jack-Burnett - Thanks very much for this contribution! @edgarrmondragon is primary reviewer for this. Since this is a non-trivial change, it may require a few days and up to a week for review and discussion.

@edgarrmondragon - Let me know if I can assist in any way.

edgarrmondragon
edgarrmondragon approved these changes Aug 25, 2022
@edgarrmondragon edgarrmondragon self-requested a review August 25, 2022 18:18
@Jack-Burnett
Copy link
Contributor Author

Hey @edgarrmondragon, I saw you approved this but didn't merge it - just wanted to check if you wanted any changes, or is it still being reviewed? No worries either way

@cjohnhanson cjohnhanson removed their request for review November 17, 2022 22:39
tests/core/test_typing.py Outdated Show resolved Hide resolved
singer_sdk/helpers/_typing.py Outdated Show resolved Hide resolved
@edgarrmondragon edgarrmondragon changed the title fix: Make conform_record_data_types work on nested objects and arrays fix: Make conform_record_data_types work on nested objects and arrays Jan 3, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

conform_record_data_types is not recursive
4 participants