Add log streaming #207
Add log streaming #207
Conversation
| import { FunctionAppTreeItem } from '../../tree/FunctionAppTreeItem'; | ||
| import { ILogStreamTreeItem } from './ILogStreamTreeItem'; | ||
|
|
||
| export async function stopStreamingLogs(tree: AzureTreeDataProvider, node?: IAzureNode<ILogStreamTreeItem>): Promise<void> { |
There was a problem hiding this comment.
Looks like this function (and corresponding context menu item) would be visible for every Function App. Is there a way to filter the node picker and context menu presence down to Apps that have a logStream present? (I see that being done with the node.treeItem.logStream conditional - I think the UX might be cleaner though)
There was a problem hiding this comment.
It's not recommended to hide things like context-menu commands based on state. Users will get confused about why it displays sometimes and not other times. Best practice is actually to gray-out the option, but I don't think that's possible in VS Code
In any case, I'll just display a warning to the user in this case.
There was a problem hiding this comment.
Yeah, greying out an option makes waay more sense. I stand corrected. :)
| if (!node) { | ||
| node = <IAzureNode<ILogStreamTreeItem>>await tree.showNodePicker(FunctionAppTreeItem.contextValue); | ||
| } | ||
|
|
There was a problem hiding this comment.
I don't see a check to verify that the Streaming Log for the chosen node hasn't already been started. I don't see anything in the docs suggesting that createOutputChannel would perform an 'already-present' check.
There was a problem hiding this comment.
Is this fixed in this AzureTools PR?
There was a problem hiding this comment.
A combination of that PR and the commit I pushed to this PR here
Also check that application logging is enabled
2 participants
Builds will fail until the appservice package is published with logstream support: microsoft/vscode-azuretools#67
Fixes #11
Edit: More fixes for log stream in the shared package here: microsoft/vscode-azuretools#68