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

feat: Add stream status trace message #18

Merged
merged 11 commits into from Apr 12, 2023

Conversation

jdpgrailsdev
Copy link
Contributor

Adds a new trace message used to convey stream status.

I am not sure about how to ensure that this ends up in the correct protocol version, now that this has been broken out into its own repo. Please advise on any changes needed to tie this change to the correct protocol version.

@jdpgrailsdev jdpgrailsdev changed the title Add stream status trace message feat: Add stream status trace message Apr 7, 2023
Copy link
Contributor

@evantahler evantahler left a comment

Choose a reason for hiding this comment

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

👍 from me on the changes to protocol-models/src/main/resources/airbyte_protocol/airbyte_protocol.yaml But I don't know what is supposed to happen with the V1 file

@evantahler
Copy link
Contributor

Something is up with the python files too:

pydantic.error_wrappers.ValidationError: 1 validation error for JsonSchemaObject
required
  value is not a valid list (type=type_error.list)

Copy link
Contributor

@davinchia davinchia left a comment

Choose a reason for hiding this comment

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

:shipit:

not familiar with the other process pieces to publish this so can't comment there.

@jdpgrailsdev
Copy link
Contributor Author

@evantahler @gosusnp @davinchia After speaking with @cgardens, I have updated this change to also include an optional stream descriptor in the AirbyteErrorTraceMessage. We will need this to ensure that we know which stream may have caused an error when a failure occurs in the destination. It has been added as an optional field, so it should be non-breaking/applied to both v0 and v1.

I am also not sure what the next step is to get this merged and released, once approved. There appears to be some issues with the CI build and I am not that familiar with this module to know if this is just noise or will also fail on merge.

@jdpgrailsdev
Copy link
Contributor Author

jdpgrailsdev commented Apr 11, 2023

Something is up with the python files too:

pydantic.error_wrappers.ValidationError: 1 validation error for JsonSchemaObject
required
  value is not a valid list (type=type_error.list)

@evantahler As far as I can tell, this error is happening before it even attempts to do the code generation. It's some failure attempting to install the datamodel_code_generator package. There are echo statements in the script and those do not appear in the output, meaning that it appears to be dying on this line: pip install datamodel_code_generator==0.11.19. For what it's worth, 0.17.2 is the latest version of that module.

@jdpgrailsdev
Copy link
Contributor Author

Something is up with the python files too:

pydantic.error_wrappers.ValidationError: 1 validation error for JsonSchemaObject
required
  value is not a valid list (type=type_error.list)

@evantahler As far as I can tell, this error is happening before it even attempts to do the code generation. It's some failure attempting to install the datamodel_code_generator package. There are echo statements in the script and those do not appear in the output, meaning that it appears to be dying on this line: pip install datamodel_code_generator==0.11.19. For what it's worth, 0.17.2 is the latest version of that module.

@evantahler Disregard. I figured it out. We don't have the --debug option on for the codegen, so we don't actually see what failed validation. Turned it on and found an issue where spotless removed my list syntax. Will fix and hopefully that gets the build to go green.

@jdpgrailsdev jdpgrailsdev merged commit 838b7ac into main Apr 12, 2023
7 checks passed
@jdpgrailsdev jdpgrailsdev deleted the jonathan/stream-status-trace-message branch April 12, 2023 18:39
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.

None yet

3 participants