cleanup(generator): api version error handling #14512
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #14512 +/- ##
==========================================
- Coverage 93.59% 93.59% -0.01%
==========================================
Files 2316 2316
Lines 207105 207100 -5
==========================================
- Hits 193838 193833 -5
Misses 13267 13267 ☔ View full report in Codecov by Sentry. |
| absl::StrCat("Unrecognized API version in package name: ", | ||
| method.file()->package()), | ||
| GCP_ERROR_INFO().WithMetadata("method", method.full_name())); | ||
| GCP_LOG(FATAL) << "Unrecognized API version in package name: " |
There was a problem hiding this comment.
I realize you are moving the GCP_LOG(FATAL) from a different location, but these are a terrible idea in the generator.
The generator runs N threads at a time. This will kill all the threads, not just the one that failed. It will leave many many files in a broken state. I think error handling in the generator needs rethinking, or maybe the idea of using one giant configuration file to call N threads needs rethinking (related to #14448).
| method.file()->package()), | ||
| GCP_ERROR_INFO().WithMetadata("method", method.full_name())); | ||
| GCP_LOG(FATAL) << "Unrecognized API version in package name: " | ||
| << method.file()->package() |
There was a problem hiding this comment.
Is the filename available? If so, please log that, and the line number. That is easier to find than package names.
Part of the work for #14510
In practice, this function does not return errors. If it did return an error, we would generate code that doesn't compile, or abort. So just move the abort into the function, and simplify the return type.
This change is