Add ExportOutput read action#261
Conversation
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| message ExportCSVOutput { | ||
| ExportCSVSource csv_source = 1; | ||
| CSVConfig csv_config = 2; | ||
| } | ||
|
|
||
| message ExportOutput { | ||
| string name = 1; | ||
|
|
||
| // Supports export formats e.g., CSV and Iceberg, but considered to be part of txn results | ||
| oneof export_output { | ||
| ExportCSVOutput csv = 2; | ||
| } | ||
| } |
There was a problem hiding this comment.
Could we not just reuse ExportCSVConfig instead of introducing ExportCSVOutput?
There was a problem hiding this comment.
ExportCSVConfig has a path and also a bunch of legacy options that we shouldn't need to support going forward. We could reuse it, but I thought it'd be cleaner to just start fresh.
There was a problem hiding this comment.
Hm I think it's fine to have an assertion that the ExportCSVConfig wrapped in a ExportOutput never uses the deprecated fields. But given that goal is to unify export and output handling, I'd prefer to keep new structures to a minimum.
There was a problem hiding this comment.
I'm even wondering whether we shouldn't just have a include_in_outputs flag on the existing Export action?
2 participants
This PR adds
ExportOutput, which is kinda halfway in betweenExportandOutput. Currently, onlyExportCSVOutputis supported, which indicates that results should be written in CSV format (similar toExport). However, the action does not specify a path, and instead it's treated as a transaction output by the engine, and written to whatever path the engine deems appropriate (similar toOutput).