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
Add CLI exit event #6142
Add CLI exit event #6142
Conversation
✅ Deploy Preview for meltano canceled.
|
Codecov Report
@@ Coverage Diff @@
## main #6142 +/- ##
==========================================
- Coverage 90.65% 90.60% -0.05%
==========================================
Files 267 267
Lines 18732 18750 +18
==========================================
+ Hits 16982 16989 +7
- Misses 1750 1761 +11
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple small changes requested.
If no other major changes, you can merge after those (and after your own testing).
If other large factors are changed/added, please let me know and I'll do another review round.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Thanks!
* Centralize schema constants into `meltano.core.tracking.schemas` * Add `IgluSchema` dataclass
Apologies for merging #6146 into this. I forgot that it wasn't targeting main. |
@WillDaSilva - not a problem. It happens. I like what you updated there so it shouldn't be an issue. |
I've tested this manually using Snowplow Micro. We could add automated testing for it by checking if Meltano subprocesses that exit with various exit codes behave properly. We'll probably want to
resolve
#2955 prior to adding such tests, as otherwise actually verifying the behaviour will be tricky, regardless of whether it works or not.(
resolve
is in backticks to stop GitHub from automatically closing #2955)Closes #5986