add PostgreSqlKernel
resolves: #3056
dup: #3676
Unable to build and test because:
./bulid.sh
❯ ./build.sh
Building NPM in directory src/polyglot-notebooks
~/dev/interactive/src/polyglot-notebooks ~/dev/interactive
npm WARN deprecated [email protected]: Please upgrade to version 7 or higher. Older versions may use Math.random() in certain circumstances, which is known to be problematic. See https://v8.dev/blog/math-random for details.
added 343 packages, and audited 344 packages in 21s
48 packages are looking for funding
run `npm fund` for details
5 vulnerabilities (2 moderate, 3 high)
To address issues that do not require attention, run:
npm audit fix
To address all issues, run:
npm audit fix --force
Run `npm audit` for details.
> @microsoft/[email protected] compile
> npm run lint && tsc -p ./ && npm run compile-library && npm run compile-es-module
> @microsoft/[email protected] lint
> eslint src --ext ts
> @microsoft/[email protected] compile-library
> rollup -c rollup.library.config.js -i src/index.ts -o lib/polyglot-notebooks.js
src/index.ts → lib/polyglot-notebooks.js...
(!) Circular dependency
src/kernel.ts -> src/kernelInvocationContext.ts -> src/kernel.ts
created lib/polyglot-notebooks.js in 6.1s
> @microsoft/[email protected] compile-es-module
> rollup -c rollup.es.config.js -i ./src/webview/activation.ts -o ./dist/activation.js
./src/webview/activation.ts → ./dist/activation.js...
(!) Circular dependency
src/kernel.ts -> src/kernelInvocationContext.ts -> src/kernel.ts
created ./dist/activation.js in 6.4s
~/dev/interactive
Building NPM in directory src/polyglot-notebooks-browser
~/dev/interactive/src/polyglot-notebooks-browser ~/dev/interactive
added 266 packages, and audited 267 packages in 21s
35 packages are looking for funding
run `npm fund` for details
5 high severity vulnerabilities
To address issues that do not require attention, run:
npm audit fix
To address all issues (including breaking changes), run:
npm audit fix --force
Run `npm audit` for details.
> [email protected] compile
> npm run rollup
> [email protected] rollup
> npm run compile-ci -- -i src/index.ts -o dist/dotnet-interactive.js
> [email protected] compile-ci
> rollup -c rollup.config.js --bundleConfigAsCjs -i src/index.ts -o dist/dotnet-interactive.js
src/index.ts → dist/dotnet-interactive.js...
[!] (plugin rpt2) RollupError: src/index.ts:3:15 - error TS2307: Cannot find module './polyglot-notebooks/commandsAndEvents' or its corresponding type declarations.
3 export * from "./polyglot-notebooks/commandsAndEvents";
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/index.ts
at error (C:\Users\Oleksii_Nikiforov\dev\interactive\src\polyglot-notebooks-browser\node_modules\rollup\dist\shared\rollup.js:349:30)
at Object.error (C:\Users\Oleksii_Nikiforov\dev\interactive\src\polyglot-notebooks-browser\node_modules\rollup\dist\shared\rollup.js:1703:20)
at RollupContext.error (C:\Users\Oleksii_Nikiforov\dev\interactive\src\polyglot-notebooks-browser\node_modules\rollup-plugin-typescript2\src\context.ts:35:17)
at C:\Users\Oleksii_Nikiforov\dev\interactive\src\polyglot-notebooks-browser\node_modules\rollup-plugin-typescript2\src\diagnostics.ts:70:17
at Array.forEach (<anonymous>)
at printDiagnostics (C:\Users\Oleksii_Nikiforov\dev\interactive\src\polyglot-notebooks-browser\node_modules\rollup-plugin-typescript2\src\diagnostics.ts:42:14)
at typecheckFile (C:\Users\Oleksii_Nikiforov\dev\interactive\src\polyglot-notebooks-browser\node_modules\rollup-plugin-typescript2\src\index.ts:67:3)
at Object.<anonymous> (C:\Users\Oleksii_Nikiforov\dev\interactive\src\polyglot-notebooks-browser\node_modules\rollup-plugin-typescript2\src\index.ts:260:5)
at Generator.next (<anonymous>)
at C:\Users\Oleksii_Nikiforov\dev\interactive\src\polyglot-notebooks-browser\node_modules\rollup-plugin-typescript2\node_modules\tslib\tslib.es6.js:76:71
It looks like you need to run this script in an elevated prompt: https://github.com/dotnet/interactive/blob/main/src/ensure-symlinks.ps1.
@NikiforovAll Could you add some tests? Like the MSSQL tests, they can be gated by a conditional FactAttribute-derived type (similar to MsSqlFactAttribute) since we don't currently run database integration tests in CI.
@NikiforovAll Are you planning to add tests? Do you need help with this PR?
@jonsequitur Yes, sorry for the late reply. I plan to do it in one or two days.
TODO:
- Install something like https://github.com/lorint/AdventureWorks-for-Postgres and test the test
- Add more tests
Install something like https://github.com/lorint/AdventureWorks-for-Postgres and test the test
We currently don't automate this kind of integration test in our CI which is why for example our MSSQL tests are only run manually. So no need to include this step in the PR, in case that was what you had in mind.
I would like to set up a test database to run tests locally.
I'm happy to try the same setup and verify. Please just include instructions on how to do that in the dev guide. (I should add some as well for the SQL tests, now that I mention it.)
@jonsequitur I've added instructions and tests. It seems like everything works as expected
Could you please assist with failing CI?
You'll need to rebase and account for this API which was renamed.
Build FAILED.
D:\a\_work\1\s\src\Microsoft.DotNet.Interactive.PostgreSql\PostgreSqlKernel.cs(115,16): error CS1061: 'CompositeKernel' does not contain a definition for 'AddKernelConnector' and no accessible extension method 'AddKernelConnector' accepting a first argument of type 'CompositeKernel' could be found (are you missing a using directive or an assembly reference?) [D:\a\_work\1\s\src\Microsoft.DotNet.Interactive.PostgreSql\Microsoft.DotNet.Interactive.PostgreSql.csproj]
0 Warning(s)
1 Error(s)
@jonsequitur rebased and fixed. Locally I managed to run the tests
This is looking great @NikiforovAll! I have one minor rename suggestion and otherwise it looks ready to merge.
@jonsequitur renamed