Skip to content
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

Removed cache_dir field #667

Merged
merged 6 commits into from
Dec 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
*Note: Numbers like (\#123) point to closed Pull Requests on the fractal-web repository.*

# Unreleased

* Removed usage of `cache_dir` field (\#667);

# 1.11.1

* Added tasks search filters (\#658);
Expand Down
13 changes: 3 additions & 10 deletions __tests__/v2/UserEditor.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ describe('UserEditor', () => {
slurm_accounts: [],
project_dir: null,
slurm_user: null,
cache_dir: null,
ssh_host: null,
ssh_username: null,
ssh_private_key_path: null,
Expand Down Expand Up @@ -80,15 +79,13 @@ describe('UserEditor', () => {
resolve({
...initialSettings,
slurm_user: 'user',
cache_dir: '/path/to/cache/dir',
project_dir: '/path/to/project/dir',
})
)
});

await user.type(screen.getByRole('textbox', { name: 'Project dir' }), '/path/to/project/dir');
await user.type(screen.getByRole('textbox', { name: 'SLURM user' }), 'user');
await user.type(screen.getByRole('textbox', { name: 'Cache dir' }), '/path/to/cache/dir');
await user.click(screen.getByRole('button', { name: 'Save' }));
await screen.findByText('User successfully updated');

Expand All @@ -99,7 +96,6 @@ describe('UserEditor', () => {
slurm_accounts: [],
project_dir: '/path/to/project/dir',
slurm_user: 'user',
cache_dir: '/path/to/cache/dir',
ssh_host: null,
ssh_username: null,
ssh_private_key_path: null,
Expand Down Expand Up @@ -130,7 +126,7 @@ describe('UserEditor', () => {
resolve({
detail: [
{
loc: ['body', 'cache_dir'],
loc: ['body', 'project_dir'],
msg: 'mocked_error',
type: 'value_error'
}
Expand All @@ -139,7 +135,7 @@ describe('UserEditor', () => {
)
});

await user.type(screen.getByRole('textbox', { name: 'Cache dir' }), 'xxx');
await user.type(screen.getByRole('textbox', { name: 'Project dir' }), 'xxx');
await user.click(screen.getByRole('button', { name: 'Save' }));
await screen.findByText('mocked_error');

Expand All @@ -148,9 +144,8 @@ describe('UserEditor', () => {
expect.objectContaining({
body: JSON.stringify({
slurm_accounts: [],
project_dir: null,
project_dir: 'xxx',
slurm_user: null,
cache_dir: 'xxx',
ssh_host: null,
ssh_username: null,
ssh_private_key_path: null,
Expand Down Expand Up @@ -204,7 +199,6 @@ describe('UserEditor', () => {
slurm_accounts: [],
project_dir: null,
slurm_user: null,
cache_dir: null,
ssh_host: 'localhost',
ssh_username: 'username',
ssh_private_key_path: 'xxx',
Expand Down Expand Up @@ -274,7 +268,6 @@ describe('UserEditor', () => {
slurm_accounts: [],
project_dir: null,
slurm_user: null,
cache_dir: null,
ssh_host: 'localhost',
ssh_username: 'username',
ssh_private_key_path: '/path/to/private/key',
Expand Down
18 changes: 9 additions & 9 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion playwright.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ export default defineConfig({

webServer: [
{
command: './tests/start-test-server.sh 2.9.0',
command: './tests/start-test-server.sh 2.9.2',
port: 8000,
waitForPort: true,
stdout: 'pipe',
Expand Down
19 changes: 0 additions & 19 deletions src/lib/components/v2/admin/UserSettingsEditor.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
});

const settingsFormErrorHandler = new FormErrorHandler('genericSettingsError', [
'cache_dir',
'slurm_accounts',
'project_dir',
'slurm_user',
Expand Down Expand Up @@ -113,24 +112,6 @@
<span class="invalid-feedback">{$settingsValidationErrors['slurm_user']}</span>
</div>
</div>
<div class="row mb-3 has-validation">
<label for="cacheDir" class="col-sm-3 col-form-label text-end">
<strong>Cache dir</strong>
</label>
<div class="col-sm-9">
<input
type="text"
class="form-control"
id="cacheDir"
bind:value={settings.cache_dir}
class:is-invalid={settingsFormSubmitted && $settingsValidationErrors['cache_dir']}
/>
<div class="form-text">
Absolute path to a user-owned folder that will be used as a cache for job-related files
</div>
<span class="invalid-feedback">{$settingsValidationErrors['cache_dir']}</span>
</div>
</div>
{:else if runnerBackend === 'slurm_ssh'}
<div class="row mb-3 has-validation">
<label for="sshHost" class="col-sm-3 col-form-label text-end">
Expand Down
42 changes: 19 additions & 23 deletions src/lib/components/v2/tasks/FilteredTasksTable.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@
<th>Modality</th>
<th>Input Types</th>
<th>Metadata</th>
<th>Version</th>
<th colspan="2">Version</th>
<slot name="extra-columns-header" />
</tr>
</thead>
Expand Down Expand Up @@ -402,27 +402,23 @@
{getMetadataCell(task.taskVersions[task.selectedVersion])}
</td>
<td class="version-col">
<div class="d-flex align-items-center">
<div class="">
{#if Object.keys(task.taskVersions).length > 1}
<select
class="form-select"
aria-label="Version for task {task.taskVersions[task.selectedVersion]
.task_name}"
bind:value={task.selectedVersion}
>
{#each sortVersions(Object.keys(task.taskVersions)) as version}
<option value={version}>{version || 'None'}</option>
{/each}
</select>
{:else}
{task.taskVersions[task.selectedVersion].version}
{/if}
</div>
<div>
<slot name="docs-info" task={task.taskVersions[task.selectedVersion]} />
</div>
</div>
{#if Object.keys(task.taskVersions).length > 1}
<select
class="form-select"
aria-label="Version for task {task.taskVersions[task.selectedVersion]
.task_name}"
bind:value={task.selectedVersion}
>
{#each sortVersions(Object.keys(task.taskVersions)) as version}
<option value={version}>{version || 'None'}</option>
{/each}
</select>
{:else}
{task.taskVersions[task.selectedVersion].version}
{/if}
</td>
<td class="docs-info-col">
<slot name="docs-info" task={task.taskVersions[task.selectedVersion]} />
</td>
<slot name="extra-columns" task={task.taskVersions[task.selectedVersion]} />
</tr>
Expand Down Expand Up @@ -457,7 +453,7 @@
}

.version-col {
max-width: 100px;
max-width: 90px;
}

.input-type-text {
Expand Down
1 change: 0 additions & 1 deletion src/lib/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,6 @@ export type UserSettings = {
project_dir: string | null
// Slurm
slurm_user: string | null
cache_dir: string | null
// Slurm SSH
ssh_host: string | null
ssh_username: string | null
Expand Down
29 changes: 0 additions & 29 deletions src/routes/settings/+page.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,6 @@
let slurmAccounts = [];
let slurmAccountsError = '';

let cacheDir = '';
let cacheDirError = '';

let settingsUpdatedMessage = '';

function addSlurmAccount() {
Expand All @@ -42,15 +39,11 @@
}
settingsUpdatedMessage = '';
slurmAccountsError = '';
cacheDirError = '';
const headers = new Headers();
headers.set('Content-Type', 'application/json');
const payload = {
slurm_accounts: slurmAccounts
};
if ($page.data.runnerBackend === 'slurm') {
payload.cache_dir = cacheDir;
}
const response = await fetch(`/api/auth/current-user/settings`, {
method: 'PATCH',
credentials: 'include',
Expand All @@ -69,10 +62,6 @@
slurmAccountsError = errorMap['slurm_accounts'];
errorShown = true;
}
if ('cache_dir' in errorMap) {
cacheDirError = errorMap['cache_dir'];
errorShown = true;
}
}
if (!errorShown) {
errorAlert = displayStandardErrorAlert(
Expand All @@ -88,7 +77,6 @@
*/
function initFields(settings) {
slurmAccounts = settings.slurm_accounts;
cacheDir = settings.cache_dir || '';
}

onMount(() => {
Expand All @@ -112,23 +100,6 @@
{settings.slurm_user || '-'}
</div>
</div>
<div class="row mb-3">
<label class="col-lg-2 col-sm-4 fw-bold" for="cache-dir">Cache dir</label>
<div class="col-lg-6 col-sm-8">
<div class="input-group" class:has-validation={cacheDirError}>
<input
type="text"
class="form-control"
id="cache-dir"
bind:value={cacheDir}
class:is-invalid={cacheDirError}
/>
{#if cacheDirError}
<div class="invalid-feedback">{cacheDirError}</div>
{/if}
</div>
</div>
</div>
{/if}
{#if $page.data.runnerBackend === 'slurm_ssh'}
<div class="row mb-4">
Expand Down
1 change: 0 additions & 1 deletion src/routes/v2/admin/groups/[groupId]/edit/+page.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,6 @@
slurm_accounts: [],
project_dir: '',
slurm_user: '',
cache_dir: '',
ssh_host: '',
ssh_username: '',
ssh_private_key_path: '',
Expand Down
4 changes: 0 additions & 4 deletions src/routes/v2/admin/users/[userId]/+page.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,6 @@
<th>SLURM user</th>
<td>{settings.slurm_user || '-'}</td>
</tr>
<tr>
<th>Cache dir</th>
<td>{settings.cache_dir || '-'}</td>
</tr>
{/if}
{#if runnerBackend === 'slurm_ssh'}
<tr>
Expand Down
19 changes: 1 addition & 18 deletions tests/settings.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,24 +38,8 @@ test('User settings', async ({ page }) => {
await page.getByText('`slurm_accounts` list has repetitions').waitFor();
});

await test.step('Add SLURM account and cache dir (success)', async () => {
await test.step('Add SLURM account (success)', async () => {
await page.getByLabel('Remove SLURM account').last().click();
await page.getByRole('textbox', { name: 'Cache dir' }).fill('/tmp');
await page.getByRole('button', { name: 'Save' }).click();
await expect(page.getByText('User settings successfully updated')).toBeVisible();
});

await test.step('Attempt to set cache dir to non absolute path', async () => {
await page.getByRole('textbox', { name: 'Cache dir' }).fill('xxx');
await page.getByRole('button', { name: 'Save' }).click();
await page
.getByText("String attribute 'cache_dir' must be an absolute path (given 'xxx').")
.waitFor();
await expect(page.getByText('`slurm_accounts` list has repetitions')).not.toBeVisible();
});

await test.step('Remove cache dir', async () => {
await page.getByRole('textbox', { name: 'Cache dir' }).fill('');
await page.getByRole('button', { name: 'Save' }).click();
await expect(page.getByText('User settings successfully updated')).toBeVisible();
});
Expand All @@ -64,7 +48,6 @@ test('User settings', async ({ page }) => {
await page.reload();
await waitPageLoading(page);
await expect(page.getByLabel('SLURM account 1')).toHaveValue(randomSlurmAccount);
await expect(page.getByRole('textbox', { name: 'Cache dir' })).toHaveValue('');
});

await test.step('Logout test user', async () => {
Expand Down
4 changes: 0 additions & 4 deletions tests/v2/admin_groups.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,6 @@ test('Admin groups management', async ({ page }) => {
await waitPageLoading(page);
await page.getByRole('textbox', { name: 'Project dir' }).fill('/tmp/test/project-dir');
await page.getByRole('textbox', { name: 'SLURM user' }).fill('test-slurm-user');
await page.getByRole('textbox', { name: 'Cache dir' }).fill('/tmp/test/cache-dir');
await page.getByRole('button', { name: 'Add SLURM account' }).click();
await page.getByLabel('SLURM account #1', { exact: true }).fill('test-slurm-account');
await page.getByRole('button', { name: 'Save' }).nth(1).click();
Expand All @@ -136,9 +135,6 @@ test('Admin groups management', async ({ page }) => {
'/tmp/test/project-dir'
);
await expect(page.getByRole('textbox', { name: 'SLURM user' })).toHaveValue('test-slurm-user');
await expect(page.getByRole('textbox', { name: 'Cache dir' })).toHaveValue(
'/tmp/test/cache-dir'
);
await expect(page.getByLabel('SLURM account #1', { exact: true })).toHaveValue(
'test-slurm-account'
);
Expand Down
Loading