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(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
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: _parse_timestamps_in_record throws exception for key not present in schema
2 participants