* Mount delete, refresh and default to new folder for mount create
- Delete mount action added
- Refresh mount action added
- Added "mymount" to theend of existing path so that we don't use already-existing HDFS folder. The call fails unless folder doesn't exist
Implemented in this PR
- New base dialog for anything needing to work with the controller. This is important since going from SQL -> Controller we "should" have the right permissions but aren't guaranteed
- Support for Mount HDFS via a dialog. Includes basic polling for success/failure, but have to give up after 2.5min as mounting could take hours. By then it's assumed to be successful since server-side has 2min timeout built in.
Not implemented in this PR
- Script as Notebook button. This should convert the inputs to a set of cells in a notebook so users can run things themselves
- Updates based on PM / UX reviews. I think we'll need a round of feedback before completing this work.
This is the 1st step to supporting HDFS Tiering
Changes:
Add new mounted folder icon. Will have separate commit for file icon
Disable delete/mkdir/upload for mounted files and folders
Disable delete for root HDFS folder (this was added in error)
- Use token API to get auth token before using other endpoints. Note this needs server updates before it'll work as expected (will only checkin after verifying this)
- Add auth option in controller UI and plumb through connection save, load, and uses of the controller API
Because the swagger spec is split in 2 created new file for the 2nd swagger spec including token auth endpoints. These come from a running cluster and instructions were updated to reflect this.
New UI Changes:
- Added authentication type field with "Basic" and "Windows Authentication" as the options
- Moved error notifications to the dialog instead of separate notification window. That's the recommended pattern
- Username / password aren't required for Windows Authentication. I couldn't find a way to change required status in form container on switching from Windows => Basic Auth so have error show on clicking OK.
- Controller URL should use ":" not "," for IP:Port
- This already handles commandline which needs same logic
- Includes visual confirmation to avoid connecting by default to untrusted server
- If successful will connect & expand in Connections tree
- If failed, will show dialog with all presets
- Shows New Query by default, can add option for New Notebook instead in the future
Connection protocol is `azuredatastudio://connect?server=mysqldw.database.windows.net&database=db1&user=myusername&aad=true`
Same args as commandline, namely:
- server
- database
- user
- aad
- integrated
I have also added `provider` arg to allow non-MSSQL providers. In the future, we should update logic to prompt for install of correct extension for that provider if it's missing.
* Fix HDFS AD write support + improve error logging
- Implemented write stream piping correctly so data is paused until we're ready to push to server
- During testing, noted that things weren't awaited correctly. Tweaked some functions to improve this
* Use correct content type
* Fix HDFS with AD auth for browse, read
- HDFS now fully supports expanding nodes for all levels, including using cookie for auth
- HDFS now support reading files from HDFS
- HDFS write file is broken and will be fixed (either in PR update or separate PR)
- Removed hack to use gateway-0 instead of actual DNS name now these are supported. Needed for testing
* Fix Jupyter error using new DMV with endpoints
Partially working AD support for BDCs with some known issues
- Plumbed through kerberos support to Notebooks.
- Using "gateway-0" for service temporarily as service endpoints API doesn't yet return correct DNS name. Will update in separate PR once available
- Plumbed kerberos auth to HDFS, Spark. Only partially working as we use same token on each call
- Will fix in separate PR, as this requires a refactor of WebHDFS library. Will need to either get new token every time or set a cookie, both of which require refactors
- Fixed error when Data Service node expansion failed and blocked all OE expansion
- Support for SqlToolsService change to use new cluster endpoints DMV
- Updated API to add new endpoints field to replace IP + port
- Added logic to handle case where endpoints for Yarn, Grafana etc. are in the list
- Sort list and use expected new localized strings
- Updated SqlToolsService to include support for new DMV
- Add "gateway-0" handling in Jupyter session as workaround for lack of domain names in endpoints list
Updated existing serialization code so it actually supports serialization. Still needs work to replace the saveAs function when a QueryProvider doesn't support save as, but want to handle in separate PR.
Removed separate MainThread/ExtHostSerializationProvider code as the DataProtocol code is the right place to put this code
Plumbed support through the gridOutputComponent to use the new serialize method in the serialization provider
Refactored the resultSerializer so majority of code can be shared between both implementations (for example file save dialog -> save -> show file on completion)
* Update to latest SQLToolsService release
- Fix spark job failing when 1 or more servers are connected
and listed in getActiveConnections
- If nothing is connected, prompts to choose a server
- Added option to dropdown when 1 or more server is connected
to support picking a different connection
Root bug was trying to access "host" not "server" in options bag, which no longer exists.
* Fix#6477 controller login + fix dashboard layout
- Service endpoints shoudl be on own column, cut off smaller screen
- Controller login not working due to 404 error
This is due to a breaking API change
We have requested fixes to help mitigate need for cluster name,
but for now have a default value for this
Finally, modified code so it's easier to update swagger API
and also added instructions on how to update in future
* Replace Big Data Cluster with big data cluster
Official docs guidance is to use "big data cluster" instead of "Big Data Cluster"
* Use doublequotes and full product name
This adds SQL Server Notebook as a built-in extension
by pulling it from blob storage.
It also adds support in mssql extension for reading the contribution points from other extensions.
This will contribute troubleshooting and other books as widgets.
In this commit:
- Bundle the extension in the build
- Bundle in sql.sh / sql.bat so it appears in local testing
- Avoid installing in Stable. Should only appear in Dev/Insiders builds
- Extensions with `notebook.books` contribution point will be discovered & their books available in MSSQL
Coming later:
- Integrate this with Maddy's work to show a Notebooks widget in the SQL Server big data cluster UI
- When clause isn't supported yet for filtering. Will be done as we refactor towards more books for different server types
Issue here is that for some reason click in tree causes 2 events:
- Selection Changed
- Resource Opened
Due to this, we get the markdown event 2 times and this misbehaves.
I am not confident in changing markdown so implemented similar pattern:
- Throttle requests if they're for the same resource inside 300ms
- This means 2nd event is ignored
Testing:
- Manual, clicked a bunch of markdowns
- It "just works" for me now
- The OEAction calls through a command service implementation that loses the context object.
Directly instantiating the NewNotebookAction solves this.
Also tested NewQuery and that didn't have an issue so left as-is
* Fix#5238 Notebooks should support relative links
- Added detection of relative #links inside notebooks
- Added handling of these, at least for current notebook
Not handled: open other notebook & scroll to position.
- Fix#6221 Notebooks: Keyboard Shortcut for New Notebook has Changed.
- Use Win+Alt+N instead of Win+Shift+N
- New Grid is now "stable" (forgot to do this in last PR)
This is a staged refactor to use the exact same grid logic in the Notebook and query editors, including context menu support, font settings, and sizing logic. The goal long term is:
- As the core Query grid is updated, Notebook can benefit from the changes
- As we add in support for contributions like new buttons & actions working on the grid, can share the logic
- Ideally if and when we refactor things like the action bar for grid results, we can apply in both places though this is TBD.
Fixes a number of issues:
- Fixes#5755 Grids don't respond to font settings. @anthonydresser can we remove setting from each query results editor and just use Notebook Styles since these are global (not scoped) settings?
- Fixes#5501 Copy from grid settings.
- Fixes#4938 SQL Notebook result sets are missing the actions provide for SQL File results sets. this now has the core ability to solve this, and separate work items for specific asks (serialization, charting) are tracked.
Currently hidden:
- Save as... options in context menu
- All right toolbar actions (save as, chart).
Remaining issues to address in future commits:
- Need to implement support for serialization (#5137).
- Need to add charting support
- Need to solve the layout of buttons on the right hand side when a small number of columns are output. It doesn't look right that buttons are so far away from the results
- Will work with UX on this. For now, mitigating this by hiding all buttons, but will need to solve in the future
- Would like to make buttons contributable via extension, but need to refactor similar to ObjectExplorer context menu so that we can serialize context menu options across to extension host while still having internal actions with full support
With this change, Plotly types will be successfully rendered in a Notebook.
Currently they have a default width of 700px with a scrollbar if the window size is smaller (this matches other notebook viewers).
The Plotly library is dynamically required to avoid startup time perf hits. This is something we could look at for other components too.
- Notebooks Can be Improperly Trusted After Save + Reopen
Fix was to only save if actually trusted.
Also fixed condition where it wasn't correctly skipping if in the queue
* Fix#3439 Trusted doesn't get saved in Notebooks
The main fix is to add a memento to store trust information. This is only needed for saved files - untitled files are always trusted as the user created them.
On clicking trusted or saving a file, the trusted state is cached. In the future, we will also handle code execution here too by sending notification on snapshot state.
I found issue #5506 during testing - existing issue where we should track trusted state changing on run. In the case all cells are ran, the whole notebook should become trusted.
Finally, I did a decent amount of refactoring to move more logic to the model - removing unnecessary calls from components which duplicated model behavior, moving trust notification to the model or at least the notebook service completely.
Added tests and logging for catch handling
VSCode serialization changed rejection handling to only serialize errors, which caused things to break
- Changed to return either account info or a cancel message in the resolve
- Rewrote to use promises. Tracking how to return canceled through 4+ thenables was way trickier than just using a promise
- Updated unit tests to handle new scenario
- Tested integration tests, realized they a) didn't run and b) didn't passed.
- Added vscode dev dependency to fix run issue
- Fixed tests to account for behavior changes in tree state.
- Copied over click handler from VSCode markdown renderer
- Added logic to only support command links if trusted
Now clicking on a link will do an action or open a file.
Created #5238 to track need to support relative links which doesn't work
This fixes an issue where if you copy azdata.proposed.d.ts to
an extension project, it would fail to compile using default
tsconfig.json settings due to missing return type
* Touchbar icon support in notebooks
- updated shortcut keys to only work if notebook is active
- Added icons
- Now have 1 "add cell" icon that prompts for code/text.
This is useful as there wasn't an icon to differentiate
* Fix 2 notebook issues
- Do not create notebook model twice on start
- Do not cause disposed warnings due to markdown cell deserialization
* Fix notebook dirty on open issue
Before model is resolved we weren't getting dirty events.
Solution is to use backing text model until it's ready.
Must hook to the dirty event & notify to get the dot to appear
* Basic touchbar support
* Touchbar support for Run Cell, Run Query, Add Cell
- Add top 3 notebook commands
- Add top query command
Actions only appear on having active editor of expected type.
In order to make Notebook work as expected, added tabindex to support focusing
and hide outline to ensure it doesn't get weird blue outline on click
Note: does not have icons yet, which would be nice.
However can add in subsequent PR once this comes from UX.
This is a partial fix that lays groundwork for full "Prompt to connect" if a kernel needs a connection.
I am waiting on Yurong's refactoring of connection handling before doing any of the prompt work.
- Adds kernel metadata about whether a connection is required.
- For Jupyter, only Spark kernels are listed as requiring a connection
- If this is true and there's no active connection, will show notification and not call execute
In the future, this path will still be used if user is prompted to connect and cancels out.
The future change will be to inject a "connect" handler from notebook.component to the cell callback and use to set connection context
Add back check for textDocuments with same name, should've been there anyhow
On rehydration files show as text docs before clicking as only get
changed by customInputConverter code path.
We should look at this long term - ideally we'd update notebookDocuments
with correct values on initial start. #4958 opened to track this.
* FIX#4513 Notebook stuck at changing kernel
- Intra-provider kernel change didn't happen because we only tried changing kernel on new session creation.
- Inverted the logic (e.g. did the right thing) and renamed the method so it's clearer what we're doing & what the boolean value should be
- Manually tested all the known scenarios
* Fix#4505 Notebooks: New Notebook will not work if existing untitled notebooks are rehydrated
* Also fixes#4508
* Unify behavior across New Notebook entry points
- Use Notebook-{n} as the standard in both entry points
- Use SQL as default provider in both
- Ensure both check for other names and only use free number
* Fix#4029 Ensure changeKernels always resolves, even in error states
This is necessary to unblock reverting the kernel on canceling Python install
- startSession now correctly sets up kernel information, since a kernel is loaded there.
- Remove call to change kernel on session initialize. This isn't needed due to refactor
- Handle kernel change failure by attempting to fall back to old kernel
- ExtensionHost $startNewSession now ensures errors are sent across the wire.
- Update AttachTo and Kernel dropdowns so they handle kernel being available. This is needed since other changes mean the session is likely ready before these get going
* Fix to handle python cancel and load existing scenarios
- Made changes to handle failure flow when Python dialog is canceled
- Made changes to handle initial load fail. Kernel and Attach To dropdowns show No Kernel / None and you can choose a kernel
- Added error wrapping in ext host so that string errors make it across and aren't lost.
* Fix#4452 Notebook is reloaded with wrong kernel
- Await all extension registration before getting providers and providerId
To do this, we need to await way up in the NotebookInput, and promise the model that we'll have values eventually
- Added to both built-in themes. Long term would be good to contribute this back to Seti so it works without a carbon edit tag.
- Issue #4418 tracks remaining problem where for new notebooks it's not set initially. This is blocked by Raj's work so will update once that lands.
* Fix#4356 New Notebook from connection doesn't connect
Fix new notebook error by passing profile instead of ID.
- I could've just sent the ID over, but this fix sets the stage for disconnected connections to work (since we have enough info to properly connect).
- There's a bug in NotebookModel blocking the disconnected connection part working, but Yurong's in progress fixes will unblock this. Hence checking in as-is and working to properly unblock once that's in.
* Support connection profile in commandline service
- Added new context API for things that want to work on commandline and object explorer
- Refactored commandlineservice slightly to be async & have a simpler execution flow (far fewer if/else statements)
* Fix unit tests
- Fixed 2 issues raised by tests (sholdn't do new query if no profile passed, shouldn't error on new query failing)
- Updated unit tests to pass as expected given changes to the APIs.
The Attach To was showing only localhost for SQL, since we overrode the standard kernels from SQL with the ones from Jupyter.
Fix is to save all standard kernels.
Also, added dispose handling for some events I found during debugging and removed unused imports
- Upate vscode-nls to 4.0.0 in notebook extension. Should be fix for Insiders build failure due to localization package failing
- Updated samples to remove as vscode-nls if not needed, or update if still needed
- Updated samples using `npm audit fix`
- Fixed compile errors in all the samples
- Fixes#4129 Overlapping command help windows in notebook
- Do not show parameter hints for inactive cells, to avoid them hanging around when no longer selected
- Fixes#4116 Notebooks: Intellisense Doesn't Work using Add New Connection
- Move connect/disconnect logic to 1 place (code component) instead of 2
- Handle the case where you connect after choosing active cell. We now hook to the event and update connection
- Fix issues in sql session manager where result outputs 0 rows. This was failing to show the empty resultset contents, which is a regression vs. query editor. It also put unhandled error on the debug console
- Fix#3913 Notebook: words selected in other cells should be unselected on cell change
Note: after fix, now looks as follows. Need to do follow up to get correct grid min height
- Jupyter completion item support was awaiting info before responding. Fix is to check if this is even a notebook cell first, then only await stuff if that's true
* Move to using notebook language by default, with override in cell
* Update cell language on kernel change
* Tweak language logic so that it prefers code mirror mode, then falls back since this was failing some notebooks
* Add new package.json contribution to define language magics. These result in cell language changing. Language is cleared out on removing the language magic
* Added support for executing Python, R and Java in the SQL Kernel to prove this out. It converts to the sp_execute_external_script format
TODO in future PR:
* Need to hook up completion item support for magics (issue #4078)
* Should add indicator at the bottom of a cell when an alternate language has been detected (issue #4079)
* On executing Python, R or Java, should add some output showing the generated code (issue #4080)
* Updated version
* On building found it had build break due to unused imports. Turns out none of the code is used (it's just a package.json + SQL file extension) so removed it all
* Removed all unnecessary node module imports, reducing size from 5.4MB to 86KB. We should probably do this for all extensions
* Fix error where kernel name was compared to itself. This doesn't break anything right now since we happen to have special handling of Python3, and for other kernels they share the same set of supported providers (which is what the check is used for). However it needs fixing for next release.
* Fix console error due to queryTextEditor trying to access model before it's ready
* Fix issues where notebook model hooked to session events multiple times
* Removed calls that weren't needed such as loadActiveContexts (if undefined, does nothing) and passing connection to initialize method
- Fix#3844
- Fix#3955
- Specs can be null on early load of Jupyter kernels
- Cells were missing in some reference test .ipynb files. We should be resilient to malformed files if possible.
Fixes#3843. Now includes full fix which limits length and ensures a scrollbar is available
- Set max size for editor. 4000px gets us 200-250 lines before needing a scrollbar.
- Adds layout updating which should also ensure accurage line highlighting to the right of the editor. What's happening is initial size is slightly off, so need to layout a 2nd time (e.g. layout once, let flex figure things out, then layout a 2nd time). This isn't optimal as there's a minor perf hit but it isn't noticeable overall.
To consider in future PRs:
- Add user configurable setting for max length?
- Handle case where we scroll to bottom but scrollbar is at the top.
- Consider how intellisense will work on this. We may need to split into a window around the current code when sending to the kernel as it's quite likely that doing a 12K line intellisense request will be too big.
* Fix#3985 Hide cell toolbar for markdown cells
* Note that I'm still hiding the overall toolbar section per UX feedback
* Also now hiding line numbers per UX feedback..
- This is a recommended update, let me know if you have concerns
- Updated all samples and Azure Data Studio specific extensions with lodash dependency
Fixes#3931
- Align run button correctly so it's centered in new cell
- Refactor to support multi-state button.
- Hidden state is set to show execution count
- Stopped state shows run button
- Running state shows stop button
- Error state (will) show error button. This isn't fully handled right now
- Add execution count to model and to SqlKernel, verify serialization, loading, update matches other notebook viewers
**Notes on implementation**:
I think this is a decent solution for a) showing execution count as text, and b) perfectly centering the run button.
The below solution shows count correctly up to 999 runs (that’s clicking 999 times in a single session), the icon lines up juuust about right with [ ] but for other numbers it is pretty close but probably not exactly right. I wish I could solve this to work better but trying to change text float to center etc. really isn’t working.
**Screenshots**:
With running cell:
- Handles empty file scenario, with fixes along the way for missing metadata (bonus win)
- In non-empty file still shows error and kernel stuck in loading state. #3964 opened to track this issue and fix later
- Fixed so it's now invisible instead of empty when not selected.
- This fixes clickability and issue where it stayed visible in 1 fix
- Also fixed cell output action which used active cell instead of context cell.
This was reviewed / worked on with Smitha and will be signed off on by PM via mail.
1 thing left (make run button look better when not selected) will be one in separate review.
Changes
- Add top/bottom padding to editor so it's not cramped
- Added an (on by default) setting `notebook.overrideEditorTheming`. This controls whether new colors etc. are used for notebook editors or if users should see vanilla UI like in standard editor. Settings under this flag are:
- When unselected, editor has same color as toolbar. On selection it goes back to regular editor view so colors work "right"
- In standard light/dark themes we now use a filled in background color instead of border box.
* Added hover support, adding box shadow and light outline on hovering and the "more actions" button showing on hover
* Added box shadow for dark themes (hooray!)
* Remove border from everything but the code cell unless a cell is selected or hovered over. This ensures this looks like a document
* Fix high contrast theming issues.
- SQLKernel is the only place to listen for batch and query complete messages now
- It routes to the 1 and only future (since can only have 1 at a time
- It handles query cancelation and not-connected issues correctly
- Editor layout gets called sometimes when other events happen (and Notebook isn't visible)
- Add in a layout call on re-setting input so the cell is updated. This fixes the problem by laying out once the UI is visible again.
Note: long term, should really be destroying the UI (while preserving the model), then restoring it including scroll selection etc. and hooking back up to the model. That is... much more work, but something we'll need long term to avoid issues where we have many Notebooks open at once. Not in scope for this PR
- Toolbar background is now differentiated from the editor
- For unselected cells there's no longer a line selection in the cell. This makes it clearer what the active cell is (and cleans the UI up)
- Added `runCell` API. Updated runCell button to listen to events on the model so it'll reflect run cell when called from other sources
- Plumbed through kernelspec info to the extension side so when changed, it's updated
- Fixed bug in ConnectionProfile where it didn't copy from options but instead overrode with empty wrapper functions
Here's the rough test code (it's in the sql-vnext extension and will be out in a separate PR)
```ts
it('Should connect to local notebook server with result 2', async function() {
this.timeout(60000);
let pythonNotebook = Object.assign({}, expectedNotebookContent, { metadata: { kernelspec: { name: "python3", display_name: "Python 3" }}});
let uri = writeNotebookToFile(pythonNotebook);
await ensureJupyterInstalled();
let notebook = await sqlops.nb.showNotebookDocument(uri);
should(notebook.document.cells).have.length(1);
let ran = await notebook.runCell(notebook.document.cells[0]);
should(ran).be.true('Notebook runCell failed');
let cellOutputs = notebook.document.cells[0].contents.outputs;
should(cellOutputs).have.length(1);
let result = (<sqlops.nb.IExecuteResult>cellOutputs[0]).data['text/plain'];
should(result).equal('2');
try {
// TODO support closing the editor. Right now this prompts and there's no override for this. Need to fix in core
// Close the editor using the recommended vscode API
//await vscode.commands.executeCommand('workbench.action.closeActiveEditor');
}
catch (e) {}
});
it('Should connect to remote spark server with result 2', async function() {
this.timeout(240000);
let uri = writeNotebookToFile(expectedNotebookContent);
await ensureJupyterInstalled();
// Given a connection to a server exists
let connectionId = await connectToSparkIntegrationServer();
// When I open a Spark notebook and run the cell
let notebook = await sqlops.nb.showNotebookDocument(uri, {
connectionId: connectionId
});
should(notebook.document.cells).have.length(1);
let ran = await notebook.runCell(notebook.document.cells[0]);
should(ran).be.true('Notebook runCell failed');
// Then I expect to get the output result of 1+1, executed remotely against the Spark endpoint
let cellOutputs = notebook.document.cells[0].contents.outputs;
should(cellOutputs).have.length(4);
let sparkResult = (<sqlops.nb.IStreamResult>cellOutputs[3]).text;
should(sparkResult).equal('2');
try {
// TODO support closing the editor. Right now this prompts and there's no override for this. Need to fix in core
// Close the editor using the recommended vscode API
//await vscode.commands.executeCommand('workbench.action.closeActiveEditor');
}
catch (e) {}
});
});
```
* Refactor JSON and format files to model and fix tabs -> spaces issues
This is in prep for some work to reuse these code files inside the model,
so pushing as its own PR to keep the next piece of work clean.
* Fix#3680 Notebooks: outputs with string arrays rendered incorrectly
- Add support for processing v4 format files loaded from disk
- Prep support for v3 notebooks by adding placeholder code for that
* Fix failing tests and add specific one for this bug
* Remove references to v5
* Handle delayed Notebook provider registration
- Fixes#3197 Notebooks: builtin provider always used on reopen with notebook file visible
- Fixes#3414 Can't refresh kernel after connect to big data cluster
There are 3 parts to this fix:
- If no notebook provider other than the default is installed, we warn users and prompt to install the SQL2019 extension
- We wait on the extension host registration to complete before determining which provider to use
- We know that the extension registration of the provider instance will be after package.json is read, so if we wait after registration for 10 seconds to give this a chance to happen before returning a provider to the front end
* Remove launch.json change that was added accidentally
* Fix timeout not being the expected value
* Removed console log left in during debugging
* Remove unnecessary whitespace
* Fix unit test failure
* Name the registration better, and remove outdated comments
- Set markdown as language for markdown cell
- Fix issue where after loading language from cell metadata, always override it. This made the "language" feature irrelevant in the cell.
- Fixed tests with new behavior (assumption: cell-level language overrides notebook-level definition) and added new test to cover this too
- Add edit API that can be used in the extension
- Separated document and editor classes out since this is the point those get big. I can refactor back in if needed to ease code review
- Based this off text editing APIs but tweaked for the fact this is a cell/array based set of edits
Kevin Cunnane
