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

SAT Testing confirms that a crash produces at least one AirbyteTraceMessage #12473

Closed
Tracked by #12322
pedroslopez opened this issue Apr 29, 2022 · 2 comments · Fixed by #12796
Closed
Tracked by #12322

SAT Testing confirms that a crash produces at least one AirbyteTraceMessage #12473

pedroslopez opened this issue Apr 29, 2022 · 2 comments · Fixed by #12796
Assignees

Comments

@pedroslopez
Copy link
Contributor

pedroslopez commented Apr 29, 2022

Full context in parent issue #12322

The SAT should verify that an AirbyteTraceMessage is emitted when something goes wrong.

This may be done by using the invalid_config.json that most connectors have and running a sync with it.

@pedroslopez pedroslopez changed the title SAT Testing confirms that a crash produces at least one <code class="notranslate">AirbyteTraceMessage` SAT Testing confirms that a crash produces at least one AirbyteTraceMessage Apr 29, 2022
@Phlair
Copy link
Contributor

Phlair commented May 3, 2022

Open Q:

  • Should we actually be testing for this is SAT if the implementation described here/here means that connectors will all be doing this automatically? In other words, this seems like a concern of the cdk implementation rather than the individual connector...
    • If we do want to have this test in SAT, then we should also create a similar issue for DAT.

@Phlair
Copy link
Contributor

Phlair commented May 3, 2022

notes from backlog grooming:

  • add this as a default test, opt-out possible for cases where cdk not being used.
    • document how to opt-out
  • clarify stance on AirbyteTraceMessage in protocol
    • strict requirement? Highly recommend? Optional?
  • create issue for DAT
  • add dependencies on cdk/java changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants