Skip to content

Commit

Permalink
Merge pull request #869 from NERC-CEH/NERCDL-1188-jupyter-basedir
Browse files Browse the repository at this point in the history
feat: revert changes for jupyter base dir
  • Loading branch information
iwalmsley authored Apr 29, 2022
2 parents 965f01a + 185b0fa commit 48abf7e
Show file tree
Hide file tree
Showing 8 changed files with 58 additions and 106 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ spec:
- name: R_LIBS_USER
value: "/data/packages/R/%p/%v"
- name: JUPYTER_DATA_DIR
value: "/data/notebooks/{{ name }}/.jupyter"
value: "/data/.jupyter"
- name: CONDA_ENV_DIR
value: "/data/conda/"
- name: JUPYTER_ALLOW_INSECURE_WRITES
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ spec:
- name: R_LIBS_USER
value: \\"/data/packages/R/%p/%v\\"
- name: JUPYTER_DATA_DIR
value: \\"/data/notebooks/deployment-name/.jupyter\\"
value: \\"/data/.jupyter\\"
- name: CONDA_ENV_DIR
value: \\"/data/conda/\\"
- name: JUPYTER_ALLOW_INSECURE_WRITES
Expand Down Expand Up @@ -426,7 +426,7 @@ spec:
- name: R_LIBS_USER
value: \\"/data/packages/R/%p/%v\\"
- name: JUPYTER_DATA_DIR
value: \\"/data/notebooks/deployment-name/.jupyter\\"
value: \\"/data/.jupyter\\"
- name: CONDA_ENV_DIR
value: \\"/data/conda/\\"
- name: JUPYTER_ALLOW_INSECURE_WRITES
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { visibility } from '../models/stackEnums';
import zeppelin from './zeppelinStack';

const { deploymentName } = nameGenerators;
const { JUPYTER, JUPYTERLAB, ZEPPELIN } = stackTypes;
const { ZEPPELIN } = stackTypes;

const mountPath = '/mnt/persistentfs';

Expand Down Expand Up @@ -100,8 +100,8 @@ export const makeZeppelinPrivate = async (name, type, projectKey) => {
await deploymentApi.restartDeployment(deployment, projectKey);
};

export const handleSharedChange = async (params, existing, newSharedStatus, userToken) => {
const { category, shared, type, volumeMount, visible } = existing;
export const handleSharedChange = async (params, existing, newSharedStatus) => {
const { category, shared, type, visible } = existing;

const oldSharedStatus = shared || visible;

Expand All @@ -126,13 +126,13 @@ export const handleSharedChange = async (params, existing, newSharedStatus, user
}

// No backend changes needed for other status changes
return;
}

if (category === NOTEBOOK_CATEGORY && newSharedStatus === visibility.PRIVATE) {
if (type === JUPYTER || type === JUPYTERLAB) {
await makeJupyterPrivate(name, type, projectKey, volumeMount, userToken);
}
// Temporarily disable Jupyter private option due to Conda issue (NERCDL-1188)
// if (type === JUPYTER || type === JUPYTERLAB) {
// await makeJupyterPrivate(name, type, projectKey, volumeMount, userToken);
// }

if (type === ZEPPELIN) {
await makeZeppelinPrivate(name, type, projectKey);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const name = 'stackname';

const volumeMount = 'volume';

const { JUPYTERLAB, ZEPPELIN, RSTUDIO, RSHINY } = stackTypes;
const { ZEPPELIN, RSTUDIO, RSHINY } = stackTypes;

const getParams = () => ({
projectKey,
Expand Down Expand Up @@ -146,43 +146,44 @@ describe('handleSharedChange', () => {
expect(ingressApi.patchIngress).toHaveBeenCalledWith('rshiny-stackname', projectKey, expectedPatch);
});

it('handles Jupyter notebooks changing to private', async () => {
const existing = {
...getExisting(),
category: NOTEBOOK_CATEGORY,
type: JUPYTERLAB,
};
jest.spyOn(Date, 'now').mockReturnValue(1609459200);

secretManager.createNewJupyterCredentials = jest.fn(() => ({ token: 'token' }));
deploymentApi.getDeployment.mockResolvedValueOnce({
spec: {
template: {
spec: {
containers: [
{
name: 'jupyterlab-stackname',
env: [
{
name: 'JUPYTER_DATA_DIR',
value: '/data/notebooks/jupyterlab-stackname/.jupyter',
},
],
},
],
},
},
},
});

await handleSharedChange(getParams(), existing, 'private', userToken);

expectCalls({ createJob: 1, createStackCredentialSecret: 1, getDeployment: 1 });
expect(secretManager.createStackCredentialSecret).toHaveBeenCalledWith(name, JUPYTERLAB, projectKey, { token: 'token' });
expect(deploymentApi.getDeployment).toHaveBeenCalledWith('jupyterlab-stackname', projectKey);
expect(jobApi.createJob).toHaveBeenCalledWith(name, projectKey, expect.any(String));
expect(jobApi.createJob.mock.calls[0][2]).toMatchSnapshot();
});
// Disable tests for making Jupyter/Zepplin private for the moment
// it('handles Jupyter notebooks changing to private', async () => {
// const existing = {
// ...getExisting(),
// category: NOTEBOOK_CATEGORY,
// type: JUPYTERLAB,
// };
// jest.spyOn(Date, 'now').mockReturnValue(1609459200);

// secretManager.createNewJupyterCredentials = jest.fn(() => ({ token: 'token' }));
// deploymentApi.getDeployment.mockResolvedValueOnce({
// spec: {
// template: {
// spec: {
// containers: [
// {
// name: 'jupyterlab-stackname',
// env: [
// {
// name: 'JUPYTER_DATA_DIR',
// value: '/data/notebooks/jupyterlab-stackname/.jupyter',
// },
// ],
// },
// ],
// },
// },
// },
// });

// await handleSharedChange(getParams(), existing, 'private', userToken);

// expectCalls({ createJob: 1, createStackCredentialSecret: 1, getDeployment: 1 });
// expect(secretManager.createStackCredentialSecret).toHaveBeenCalledWith(name, JUPYTERLAB, projectKey, { token: 'token' });
// expect(deploymentApi.getDeployment).toHaveBeenCalledWith('jupyterlab-stackname', projectKey);
// expect(jobApi.createJob).toHaveBeenCalledWith(name, projectKey, expect.any(String));
// expect(jobApi.createJob.mock.calls[0][2]).toMatchSnapshot();
// });

it('handles Zeppelin notebooks changing to private', async () => {
const existing = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export const getSharedButtons = (stack, shareStack, permission) => {
const shared = stack.shared || stack.visible;

return [
shareButton(PRIVATE, 'Set Access: Private', shared === PRIVATE),
isSite ? shareButton(PRIVATE, 'Set Access: Private', shared === PRIVATE) : undefined,
shareButton(PROJECT, 'Set Access: Project', shared === PROJECT),
isSite ? shareButton(PUBLIC, 'Set Access: Public', shared === PUBLIC) : undefined,
];
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React from 'react';
import { render, fireEvent, screen, within } from '@testing-library/react';
import { SITE_CATEGORY } from 'common/src/config/images';
import { NOTEBOOK_CATEGORY, SITE_CATEGORY } from 'common/src/config/images';
import { useCurrentUserId } from '../../../hooks/authHooks';
import StackCardActions, { getSharedButtons } from './StackCardActions';
import { getUserActionsForType } from '../../../config/images';
Expand Down Expand Up @@ -285,20 +285,24 @@ describe('getSharedButtons', () => {
});

it('creates the correct buttons for a private stack', () => {
const stack = getStack();
const stack = {
...getStack(),
category: NOTEBOOK_CATEGORY,
};
const buttons = getSharedButtons(stack, jest.fn(), permission);

expect(buttons).toEqual([privateButton(true), projectButton(false), undefined]);
expect(buttons).toEqual([undefined, projectButton(false), undefined]);
});

it('creates the correct buttons for a project stack', () => {
const stack = {
...getStack(),
shared: 'project',
category: NOTEBOOK_CATEGORY,
};
const buttons = getSharedButtons(stack, jest.fn(), permission);

expect(buttons).toEqual([privateButton(false), projectButton(true), undefined]);
expect(buttons).toEqual([undefined, projectButton(true), undefined]);
});

it('creates the correct buttons for a public stack', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,13 +102,6 @@ exports[`StackCardActions creates correct snapshot 2`] = `
Edit
disabled: undefined
</div>
<div
requiredpermission="delete"
userpermissions="open,delete,edit"
>
Set Access: Private
disabled: true
</div>
<div
requiredpermission="delete"
userpermissions="open,delete,edit"
Expand Down Expand Up @@ -146,13 +139,6 @@ exports[`StackCardActions creates correct snapshot if there are multiple copy sn
Edit
disabled: undefined
</div>
<div
requiredpermission="delete"
userpermissions="open,delete,edit"
>
Set Access: Private
disabled: true
</div>
<div
requiredpermission="delete"
userpermissions="open,delete,edit"
Expand Down

0 comments on commit 48abf7e

Please sign in to comment.