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: Handle replication key not found in stream schema #1927

Conversation

mjsqu
Copy link
Contributor

@mjsqu mjsqu commented Aug 27, 2023

Closes: #1332

Changes:

  • Add new InvalidReplicationKeyException to singer_sdk.exceptions
  • Update streams.core. A get() is used to obtain the replication_key from self.schema, if the key is not present in the schema, the variable type_dict would equal None. A check has been added, if the value of the type_dict is None then the new exception is raised
  • The resulting error message is of the format:
"incorrect_replication_key" is not in schema for stream name: my_stream"
  • Add a test to tests/core/test_streams.py which checks that a SimpleTestStream with replication_key overridden to "INVALID" raises the correct InvalidReplicationKeyException exception.

@mjsqu mjsqu changed the title Handle replication key not found in stream schema bugfix: Handle replication key not found in stream schema Aug 27, 2023
@mjsqu mjsqu changed the title bugfix: Handle replication key not found in stream schema fix: Handle replication key not found in stream schema Aug 27, 2023
@codecov
Copy link

codecov bot commented Aug 27, 2023

Codecov Report

Merging #1927 (0ca1e27) into main (4eac6f4) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1927      +/-   ##
==========================================
+ Coverage   86.96%   86.97%   +0.01%     
==========================================
  Files          59       59              
  Lines        5087     5091       +4     
  Branches      824      825       +1     
==========================================
+ Hits         4424     4428       +4     
  Misses        466      466              
  Partials      197      197              
Files Changed Coverage Δ
singer_sdk/exceptions.py 100.00% <100.00%> (ø)
singer_sdk/streams/core.py 84.07% <100.00%> (+0.11%) ⬆️

📣 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.

Thanks for contributing @mjsqu! Just a small nit.

singer_sdk/streams/core.py Outdated Show resolved Hide resolved
@edgarrmondragon
Copy link
Collaborator

Thanks @mjsqu!

@edgarrmondragon edgarrmondragon merged commit b1b97b0 into meltano:main Aug 28, 2023
@mjsqu mjsqu deleted the bugfix/improve_err_msg_replication_key_not_present branch August 28, 2023 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Improve error message when replication key is not present in schema
2 participants