-
Notifications
You must be signed in to change notification settings - Fork 506
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
reenable prelaunch-hook #724
Conversation
This is a great feature. Can maintainers of this repo comment on if this can be accepted or maybe offer alternative guidance for handling the issue @timkpaine has raised? |
Would echo the previous sentiment, I too would appreciate if the maintainers can comment on Tims' patch. |
@jtpio I think this a a good feature, should we consider it for |
Echoing the above two comments. Please advise on how I can pass a single string variable to my notebook. I am using |
Benchmark reportThe execution time (in milliseconds) are grouped by test file, test type and browser. Results table
❗ Test metadata have changed--- /dev/fd/63 2022-07-25 13:04:09.018022201 +0000
+++ /dev/fd/62 2022-07-25 13:04:09.018022201 +0000
@@ -4,37 +4,37 @@
"BENCHMARK_REFERENCE": "actual"
},
"browsers": {
- "chromium": "97.0.4666.0"
+ "chromium": "94.0.4595.0"
},
"systemInformation": {
"cpu": {
- "brand": "Xeon® Platinum 8171M",
+ "brand": "Xeon® E5-2673 v3",
"cache": {
"l1d": 65536,
"l1i": 65536,
- "l2": 2097152,
- "l3": 36700160
+ "l2": 524288,
+ "l3": 31457280
},
"cores": 2,
"family": "6",
- "flags": "fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ss ht syscall nx pdpe1gb rdtscp lm constant_tsc rep_good nopl xtopology cpuid pni pclmulqdq ssse3 fma cx16 pcid sse4_1 sse4_2 movbe popcnt aes xsave avx f16c rdrand hypervisor lahf_lm abm 3dnowprefetch invpcid_single pti fsgsbase bmi1 hle avx2 smep bmi2 erms invpcid rtm avx512f avx512dq rdseed adx smap clflushopt avx512cd avx512bw avx512vl xsaveopt xsavec xsaves md_clear",
+ "flags": "fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ss ht syscall nx pdpe1gb rdtscp lm constant_tsc rep_good nopl xtopology cpuid pni pclmulqdq ssse3 fma cx16 pcid sse4_1 sse4_2 movbe popcnt aes xsave avx f16c rdrand hypervisor lahf_lm abm invpcid_single pti fsgsbase bmi1 avx2 smep bmi2 erms invpcid xsaveopt md_clear",
"governor": "",
"manufacturer": "Intel®",
- "model": "85",
+ "model": "63",
"physicalCores": 2,
"processors": 1,
"revision": "",
"socket": "",
- "speed": 2.6,
+ "speed": 2.4,
"speedMax": null,
"speedMin": null,
- "stepping": "4",
+ "stepping": "2",
"vendor": "GenuineIntel",
"virtualization": false,
"voltage": ""
},
"mem": {
- "total": 7281008640
+ "total": 7291699200
},
"osInfo": {
"arch": "x64",
@@ -42,11 +42,11 @@
"codename": "Focal Fossa",
"codepage": "UTF-8",
"distro": "Ubuntu",
- "kernel": "5.15.0-1014-azure",
+ "kernel": "5.8.0-1040-azure",
"logofile": "ubuntu",
"platform": "linux",
- "release": "20.04.4 LTS",
- "serial": "2dc2d73cec5c409b97a391e53a41a4ba",
+ "release": "20.04.3 LTS",
+ "serial": "cfc067bfcb844f35865e279a1b0e66c5",
"servicepack": "",
"uefi": false
} |
I am getting this back up and working, is there any specific examples/docs I should add? I no longer have access to how I used this to enable custom token-based authentication at JP Morgan (maybe @vidartf can construct some examples off this) but im planning on adding a simple test to extract a query parameters and run papermill so I can close #484 |
This PR would solve my needs for authentication in a very clean way. If I want to support auth via authorization token or cookies, I could simply read them via this hook and then mutate a notebook cell to include the authorization information required to make additional client requests. Edit: Oh, I guess that's exactly how papermill works. Neat. |
Well I put the minor fix in a PR here hoping to see the workflows run: #1148 |
I will do it here tonight, apologies |
@alkasm I merged your branch into mine, as a weird side effect your PR and my PR are now both running the tests but I think they're the same |
Awesome, and no worries on the timeline. I deleted my branch. Thanks for being so responsive on a relatively stale PR! |
Wanted to note that this is NOT a breaking change, so does not necessarily need to wait for 4.0. |
Yeah it looks like flaky tests, I tried 4-5 times (rebased and force pushed empty commits) and got different tests failing every time |
@trungleduc @jtpio @maartenbreddels @martinRenou @davidbrochart ready for review / discussion |
Just bumping, I can't sponsor this feature right now with my current employer but I can reveal that a large former employer, J.P. Morgan, uses/used this feature extensively which is a decent endorsement: |
Hi @timkpaine, thank you very much for your contribution! I'm quite busy during this period but I will take a look at this PR in a few weeks. |
I've noticed that this only works when you serve a single notebook, e.g. by overriding the edit: I think probably adding it to here is sufficient: Lines 568 to 573 in e658bbe
I'll test that out shortly edit 2: Yeah, that works |
Thanks @timkpaine for your contribution, it looks really nice. |
I confirm, the |
That is correct, I can add a note to the documentation. I have long rallied against the preheat mode for this reason: #712 (comment) |
I think the documentation for this conflict is not enough, we might need some checks on startup and warn users if they use both features. |
Thanks @timkpaine! |
thanks @trungleduc ! cc @vidartf @ptomecek |
Just updated the milestone to According to the diff and the comment above. there doesn't seem to be breaking changes, so this can be shipped in the next |
A function that is called prior to the launch of a new kernel instance
when a user visits the voila webpage. Used for custom user authorization
or any other necessary pre-launch functions.
Should be of the form
Although most customizations can leverage templates, if you need access
to the request object (e.g. to inspect cookies for authentication),
or to modify the notebook itself (e.g. to inject some custom structure,
althought much of this can be done by interacting with the kernel
in javascript) the prelaunch hook lets you do that.