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(targets): Safely skip parsing record field as date-time if it is missing in schema #1844

Conversation

edgarrmondragon
Copy link
Collaborator

@edgarrmondragon edgarrmondragon commented Jul 14, 2023

Closes #1836

  • Add tests

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

@edgarrmondragon edgarrmondragon changed the title fix: Safely ignore parsing record field as datetime if it is missing in schema fix: Safely skip parsing record field as date-time if it is missing in schema Jul 14, 2023
@codecov
Copy link

codecov bot commented Jul 14, 2023

Codecov Report

Merging #1844 (20ca4b1) into main (fe7d7d6) will increase coverage by 0.22%.
The diff coverage is 83.33%.

@@            Coverage Diff             @@
##             main    #1844      +/-   ##
==========================================
+ Coverage   87.16%   87.39%   +0.22%     
==========================================
  Files          59       59              
  Lines        5120     5123       +3     
  Branches      827      828       +1     
==========================================
+ Hits         4463     4477      +14     
+ Misses        463      451      -12     
- Partials      194      195       +1     
Files Coverage Δ
singer_sdk/sinks/core.py 93.33% <83.33%> (+4.44%) ⬆️

... and 1 file with indirect coverage changes

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

@edgarrmondragon edgarrmondragon changed the title fix: Safely skip parsing record field as date-time if it is missing in schema fix(targets): Safely skip parsing record field as date-time if it is missing in schema Jul 31, 2023
@edgarrmondragon edgarrmondragon marked this pull request as ready for review September 25, 2023 17:31
Comment on lines 369 to 371
if key not in schema["properties"]:
self.logger.debug("No schema for record field '%s'", key)
continue
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Using the in operator is a bit faster than try-except:

import timeit

setup = """
big_dict = {
    i: i * i
    for i in range(50_000)
}
"""

try_except = """
for i in range(70_000):
    try:
        x = big_dict[i]
    except KeyError:
        continue
"""

in_operator = """
for i in range(70_000):
    if i in big_dict:
        x = big_dict[i]
"""

print(timeit.timeit(setup=setup, stmt=try_except, number=1000))   # 4.430988875003095
print(timeit.timeit(setup=setup, stmt=in_operator, number=1000))  # 2.137157458000729

@edgarrmondragon edgarrmondragon merged commit bfb7baa into main Sep 28, 2023
24 of 25 checks passed
@edgarrmondragon edgarrmondragon deleted the 1836-bug-_parse_timestamps_in_record-throws-exception-for-key-not-present-in-schema branch September 28, 2023 23:26
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: _parse_timestamps_in_record throws exception for key not present in schema
2 participants