RFC - [computelogmanager] add fields to pass uri, path or shell cmd #26723
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
mlarose
changed the title
| external_stdout_url=log_context.external_stdout_url, | ||
| external_stderr_url=log_context.external_stderr_url, | ||
| stdout_shell_cmd=log_context.stdout_shell_cmd, |
There was a problem hiding this comment.
is adding fields to a LOGS_CAPTURED event payload problematic?
| {externalUrl} | ||
| </a> | ||
| <Icon name="open_in_new" color={Colors.tooltipText()} size={20} style={{marginTop: 2}} /> | ||
| {externalUrl ? ( |
There was a problem hiding this comment.
this will definitely need css+html rework, i was just passing information and making the click to copy work.
when thinking about this ui, should we provide even more information, such as additional text about to use the shell command (ie, requiring a preconfigured azure cli, for example) ? I stopped short of this because I was already concerned about increasing the event size, but maybe I could add a 'log type' field that the UI could use to render that additional information without producing it from the ComputeLogManager
|
Deploy preview for dagit-core-storybook ready! ✅ Preview Built with commit ce0b204. |
Summary & Motivation
The Azure Blob Storage Container do not provide direct urls to objects on private storage accounts, even if the user is authenticated.
This PR provide new fields on the captured logs entry that such ComputeLogManager could populate to improve the user experience by providing a path or uri and/or a shell command to access those logs.
How I Tested These Changes
Changelog