Feature: Add an option to generate exceptions for Conjure errors#1306
Feature: Add an option to generate exceptions for Conjure errors#1306wsuppiger wants to merge 4 commits into
Conversation
✅ Successfully generated changelog entry!Entry generated via PR titleTo modify this entry, edit PR title using proper format. 📋Changelog Preview✨ Features
|
| namespace: Datasets | ||
| code: INVALID_ARGUMENT | ||
| safe-args: | ||
| fileSystemId: string |
There was a problem hiding this comment.
minor: Include docs for one of the args here
| .build(), | ||
| PythonImport.of("builtins"), | ||
| PythonImport.builder() | ||
| .moduleSpecifier(ImportTypeVisitor.TYPING) |
There was a problem hiding this comment.
This import will be unused if both safeArgs and unsafeArgs are empty.
| poetWriter.writeIndentedLine("return ("); | ||
| poetWriter.increaseIndent(); | ||
| poetWriter.writeIndentedLine("error.error_name == cls.ERROR_NAME and"); | ||
| poetWriter.writeIndentedLine("error.error_code == cls.ERROR_CODE"); |
There was a problem hiding this comment.
minor: I'd only check error name, for consistency with conjure-java and conjure-typescript.
| import org.immutables.value.Value; | ||
|
|
||
| @Value.Immutable | ||
| public interface ErrorSnippet extends PythonSnippet { |
There was a problem hiding this comment.
The generated error class doesn't provide a way to generate an instance from just the args (unlike conjure-java, which does). I'm assuming this is because you didn't intend for this to be used server-side, only to extract errors to a more convenient type on the client side? Is it worth generating constructors as well, and decoupling this from needing to extend ConjureHTTPError?
| default void emitConstructor(PythonPoetWriter poetWriter) { | ||
| poetWriter.writeIndentedLine("def __init__(self, base_error: ConjureHTTPError) -> None:"); | ||
| poetWriter.increaseIndent(); | ||
| poetWriter.writeIndentedLine("super().__init__("); |
There was a problem hiding this comment.
| poetWriter.writeIndentedLine("super().__init__("); | ||
| poetWriter.increaseIndent(); | ||
| poetWriter.writeIndentedLine("status_code=base_error.status_code,"); | ||
| // TODO(bzhang): Use enum once https://github.com/palantir/conjure-python-client/pull/171 is merged |
There was a problem hiding this comment.
minor: this comment shouldn't appear in the final PR; we should either merge and pick up here, or remove the comment (if we don't want to merge)
branch off: #1157
For issue: #910
After this PR
A few things happening:
The resulting error look like:
Also placed a follow-up item for using the enum for
ErrorCodeafter palantir/conjure-python-client#171.