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

Test onedata objectstore with new caching #4

Closed
wants to merge 39 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
87124a3
Don't copy from_work_dir outputs to purged path in shell fragment
mvdbeek Jun 7, 2024
c3a5d61
Don't include purged outputs in from_work_dir outputs
mvdbeek Jun 7, 2024
a1d41ab
Raise appropriate exception if require path parameter is missing
mvdbeek Jun 7, 2024
8e6e25d
Don't push purged dataset contents to object store
mvdbeek Jun 7, 2024
8bc3f21
Add tool that produces all types of outputs
mvdbeek Jun 8, 2024
65e9605
Don't fail _finish_or_resubmit if tool_stdout / tool_stderr not written
mvdbeek Jun 8, 2024
435a8cd
Fix up test cases
mvdbeek Jun 9, 2024
03eca7c
Fix handling of collecting discovered but purged outputs
mvdbeek Jun 9, 2024
eb2e318
Elevate external metadata setting to warning
mvdbeek Jun 10, 2024
fc42b61
Fix metadata setting on purged outputs
mvdbeek Jun 10, 2024
a35033d
Extend test to make sure disk path remains purged
mvdbeek Jun 10, 2024
c489eb2
Set file_size and total_size to 0 for discovered but purged datasets
mvdbeek Jun 10, 2024
af923d6
Fix anonymous user job retrieval logic
davelopez Jun 10, 2024
a6b45e5
Filter jobs by session instead of using the session current history f…
davelopez Jun 10, 2024
5ab3120
Purge dataset if finishing job for purged output
mvdbeek Jun 10, 2024
50b7f8f
Do not overwrite purged or deleted states when importing datasets fro…
mvdbeek Jun 10, 2024
3c7730c
Delete purged outputs from object store when importing dataset for pu…
mvdbeek Jun 10, 2024
9df23a6
Require session to list jobs instead of returning empty job list for …
davelopez Jun 10, 2024
d0414c3
Fix check for anonymous
jdavcs Jun 10, 2024
d7dad3a
Merge pull request #18358 from davelopez/24.0_fix_condition_anonymous…
martenson Jun 11, 2024
cea7eb0
Merge pull request #18364 from jdavcs/241_bug_reverse_bool
jdavcs Jun 11, 2024
b2bbdcb
Merge branch 'release_24.0' into release_24.1
jdavcs Jun 11, 2024
8351709
Fix mypy after merge
jdavcs Jun 11, 2024
13de716
Merge pull request #18365 from jdavcs/241_merge_240_2
jdavcs Jun 11, 2024
f0e09cc
Fix up tests
mvdbeek Jun 10, 2024
8ea34dd
✨: add `Link to workflow to` WorkflowActionsExtend
itisAliRH Jun 11, 2024
0fbd20e
🎨: replace `workflow external link` icon with `faFileImport` in `Work…
itisAliRH Jun 11, 2024
96c9be3
Merge pull request #18342 from mvdbeek/fix_copying_purged_files
mvdbeek Jun 11, 2024
b8e2bfc
Merge branch 'release_24.0' into release_24.1
mvdbeek Jun 11, 2024
c5d1231
✨: add `getFullAppUrl` utility function
itisAliRH Jun 11, 2024
7253c22
🔨: replace direct URL construction with the `getFullAppUrl` utility f…
itisAliRH Jun 11, 2024
a363a24
🐛: remove unnecessary condition for anonymous user in `WorkflowAction…
itisAliRH Jun 11, 2024
b1614ba
🚨: add unit test for `getFullAppUrl` utility function
itisAliRH Jun 11, 2024
94513b8
Merge pull request #18370 from itisAliRH/workflow-card-publish-link-copy
ahmedhamidawan Jun 11, 2024
2d65374
Replace demo.onedata.org with datahub.egi.eu
bwalkowi May 27, 2024
d718a93
Update OnedataObjectStore to new onedatafilerestclient API
bwalkowi May 27, 2024
dcc5096
Update onedatafilerestclient requirement
bwalkowi May 27, 2024
2ec50fb
Use only single filterdir in _get_total_matches_count
bwalkowi Jun 11, 2024
396c3e4
Use basic namespace when counting files with scandir
bwalkowi Jun 12, 2024
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
9 changes: 2 additions & 7 deletions client/src/components/Sharing/Embeds/WorkflowEmbed.vue
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import { useDebounce } from "@vueuse/core";
import { BButton, BFormCheckbox, BFormInput, BInputGroup, BInputGroupAppend } from "bootstrap-vue";
import { computed, reactive, ref } from "vue";

import { getAppRoot } from "@/onload/loadConfig";
import { copy } from "@/utils/clipboard";
import { getFullAppUrl } from "@/utils/utils";

import ZoomControl from "@/components/Workflow/Editor/ZoomControl.vue";
import WorkflowPublished from "@/components/Workflow/Published/WorkflowPublished.vue";
Expand Down Expand Up @@ -39,13 +39,8 @@ function onChangePosition(event: Event, xy: "x" | "y") {
}
}

const root = computed(() => {
const port = window.location.port ? `:${window.location.port}` : "";
return `${window.location.protocol}//${window.location.hostname}${port}${getAppRoot()}`;
});

const embedUrl = computed(() => {
let url = `${root.value}published/workflow?id=${props.id}&embed=true`;
let url = getFullAppUrl(`published/workflow?id=${props.id}&embed=true`);
url += `&buttons=${settings.buttons}`;
url += `&about=${settings.about}`;
url += `&heading=${settings.heading}`;
Expand Down
8 changes: 2 additions & 6 deletions client/src/components/Sharing/SharingPage.vue
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { getGalaxyInstance } from "@/app";
import { useToast } from "@/composables/toast";
import { getAppRoot } from "@/onload/loadConfig";
import { errorMessageAsString } from "@/utils/simple-error";
import { getFullAppUrl } from "@/utils/utils";

import type { Item, ShareOption } from "./item";

Expand Down Expand Up @@ -52,11 +53,6 @@ const item = ref<Item>({
extra: defaultExtra(),
});

const itemRoot = computed(() => {
const port = window.location.port ? `:${window.location.port}` : "";
return `${window.location.protocol}//${window.location.hostname}${port}${getAppRoot()}`;
});

const itemUrl = reactive({
prefix: "",
slug: "",
Expand All @@ -68,7 +64,7 @@ watch(
if (value) {
const index = value.lastIndexOf("/");

itemUrl.prefix = itemRoot.value + value.substring(0, index + 1);
itemUrl.prefix = getFullAppUrl(value.substring(0, index + 1));
itemUrl.slug = value.substring(index + 1);
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import { FontAwesomeIcon } from "@fortawesome/vue-fontawesome";
import { computed } from "vue";
import { RouterLink } from "vue-router";

import { getAppRoot } from "@/onload/loadConfig";
import { useUserStore } from "@/stores/userStore";
import { getFullAppUrl } from "@/utils/utils";

import Heading from "@/components/Common/Heading.vue";
import CopyToClipboard from "@/components/CopyToClipboard.vue";
Expand Down Expand Up @@ -42,17 +42,12 @@ const gravatarSource = computed(

const publishedByUser = computed(() => `/workflows/list_published?owner=${props.workflowInfo?.owner}`);

const root = computed(() => {
const port = window.location.port ? `:${window.location.port}` : "";
return `${window.location.protocol}//${window.location.hostname}${port}${getAppRoot()}`;
});

const relativeLink = computed(() => {
return `/published/workflow?id=${props.workflowInfo.id}`;
});

const fullLink = computed(() => {
return `${root.value}${relativeLink.value.substring(1)}`;
return getFullAppUrl(relativeLink.value.substring(1));
});

const userOwned = computed(() => {
Expand Down
40 changes: 29 additions & 11 deletions client/src/components/Workflow/WorkflowActionsExtend.vue
Original file line number Diff line number Diff line change
@@ -1,15 +1,6 @@
<script setup lang="ts">
import { library } from "@fortawesome/fontawesome-svg-core";
import { faStar as farStar } from "@fortawesome/free-regular-svg-icons";
import {
faCaretDown,
faCopy,
faDownload,
faFileExport,
faShareAlt,
faStar,
faTrashRestore,
} from "@fortawesome/free-solid-svg-icons";
import { faCopy, faDownload, faLink, faShareAlt, faTrashRestore } from "@fortawesome/free-solid-svg-icons";
import { FontAwesomeIcon } from "@fortawesome/vue-fontawesome";
import { BButton } from "bootstrap-vue";
import { storeToRefs } from "pinia";
Expand All @@ -19,9 +10,11 @@ import { copyWorkflow, undeleteWorkflow } from "@/components/Workflow/workflows.
import { useConfirmDialog } from "@/composables/confirmDialog";
import { Toast } from "@/composables/toast";
import { useUserStore } from "@/stores/userStore";
import { copy } from "@/utils/clipboard";
import { withPrefix } from "@/utils/redirect";
import { getFullAppUrl } from "@/utils/utils";

library.add(faCaretDown, faCopy, faDownload, faFileExport, faShareAlt, farStar, faStar, faTrashRestore);
library.add(faCopy, faDownload, faLink, faShareAlt, faTrashRestore);

interface Props {
workflow: any;
Expand Down Expand Up @@ -72,11 +65,36 @@ async function onRestore() {
Toast.info("Workflow restored");
}
}

const relativeLink = computed(() => {
return `/published/workflow?id=${props.workflow.id}`;
});

const fullLink = computed(() => {
return getFullAppUrl(relativeLink.value.substring(1));
});

function onCopyPublicLink() {
copy(fullLink.value);
Toast.success("Link to workflow copied");
}
</script>

<template>
<div class="workflow-actions-extend flex-gapx-1">
<BButtonGroup>
<BButton
v-if="workflow.published && !workflow.deleted"
id="workflow-copy-public-button"
v-b-tooltip.hover.noninteractive
:size="buttonSize"
title="Copy link to workflow"
variant="outline-primary"
@click="onCopyPublicLink">
<FontAwesomeIcon :icon="faLink" fixed-width />
<span class="compact-view">Link to Workflow</span>
</BButton>

<BButton
v-if="!isAnonymous && !shared && !workflow.deleted"
id="workflow-copy-button"
Expand Down
6 changes: 3 additions & 3 deletions client/src/components/Workflow/WorkflowIndicators.vue
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<script setup lang="ts">
import { library } from "@fortawesome/fontawesome-svg-core";
import { faGlobe, faLink, faShieldAlt, faUser, faUsers } from "@fortawesome/free-solid-svg-icons";
import { faFileImport, faGlobe, faShieldAlt, faUser, faUsers } from "@fortawesome/free-solid-svg-icons";
import { FontAwesomeIcon } from "@fortawesome/vue-fontawesome";
import { BBadge, BButton } from "bootstrap-vue";
import { computed } from "vue";
Expand All @@ -12,7 +12,7 @@ import { copy } from "@/utils/clipboard";

import UtcDate from "@/components/UtcDate.vue";

library.add(faShieldAlt, faLink, faGlobe, faUsers, faUser);
library.add(faFileImport, faGlobe, faShieldAlt, faUsers, faUser);

interface Props {
workflow: any;
Expand Down Expand Up @@ -116,7 +116,7 @@ function onViewUserPublished() {
size="sm"
class="workflow-external-link inline-icon-button"
:title="sourceTitle">
<FontAwesomeIcon :icon="faLink" fixed-width @click="onCopyLink" />
<FontAwesomeIcon :icon="faFileImport" fixed-width @click="onCopyLink" />
</BButton>

<span class="mr-1">
Expand Down
12 changes: 12 additions & 0 deletions client/src/utils/utils.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,4 +101,16 @@ describe("test utils", () => {
]);
});
});

describe("test getFullAppUrl", () => {
it("should return the full app url", () => {
const appUrl = Utils.getFullAppUrl();
expect(appUrl).toBe("http://localhost/");
});

it("should return the full app url", () => {
const appUrl = Utils.getFullAppUrl("home");
expect(appUrl).toBe("http://localhost/home");
});
});
});
16 changes: 16 additions & 0 deletions client/src/utils/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,21 @@ export function hasKeys(object: unknown, keys: string[]) {
}
}

/**
* Get the full URL path of the app
*
* @param path Path to append to the URL path
* @returns Full URL path of the app
*/
export function getFullAppUrl(path: string = ""): string {
const protocol = window.location.protocol;
const hostname = window.location.hostname;
const port = window.location.port ? `:${window.location.port}` : "";
const appRoot = getAppRoot();

return `${protocol}//${hostname}${port}${appRoot}${path}`;
}

export default {
cssLoadFile,
get,
Expand All @@ -460,4 +475,5 @@ export default {
waitForElementToBePresent,
wait,
mergeObjectListsById,
getFullAppUrl,
};
4 changes: 2 additions & 2 deletions lib/galaxy/config/sample/object_store_conf.sample.yml
Original file line number Diff line number Diff line change
Expand Up @@ -223,9 +223,9 @@ auth:
# an access token suitable for data access (allowing calls to the Oneprovider REST API).
access_token: ...
connection:
# the domain of the Onezone service (e.g. "demo.onedata.org"), or its IP address for
# the domain of the Onezone service (e.g. datahub.egi.eu), or its IP address for
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thx

# devel instances (see above).
onezone_domain: demo.onedata.org
onezone_domain: datahub.egi.eu
# Allows connection to Onedata servers that do not present trusted SSL certificates.
# SHOULD NOT be used unless you really know what you are doing.
disable_tls_certificate_validation: false
Expand Down
4 changes: 2 additions & 2 deletions lib/galaxy/config/sample/object_store_conf.xml.sample
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@
an access token suitable for data access (allowing calls to the Oneprovider REST API).

//connection/@onezone_domain -
the domain of the Onezone service (e.g. "demo.onedata.org"), or its IP address for
the domain of the Onezone service (e.g. datahub.egi.eu), or its IP address for
devel instances (see above).

//connection/@disable_tls_certificate_validation -
Expand All @@ -182,7 +182,7 @@
<!--
<object_store type="onedata">
<auth access_token="..." />
<connection onezone_domain="demo.onedata.org" disable_tls_certificate_validation="False"/>
<connection onezone_domain="datahub.egi.eu" disable_tls_certificate_validation="False"/>
<space name="demo-space" path="galaxy-data" />
<cache path="database/object_store_cache" size="1000" cache_updated_data="True" />
<extra_dir type="job_work" path="database/job_working_directory_onedata"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ preferences:
description: Your Onedata account
inputs:
- name: onezone_domain
label: Domain of the Onezone service (e.g. "demo.onedata.org")
label: Domain of the Onezone service (e.g. datahub.egi.eu)
type: text
required: False
- name: access_token
Expand Down
2 changes: 1 addition & 1 deletion lib/galaxy/dependencies/conditional-requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ fs.googledrivefs # type: googledrive
fs-gcsfs # type: googlecloudstorage
# fs-gcsfs doesn't pin google-cloud-storage, and old versions log noisy exceptions and break test discovery
google-cloud-storage>=2.8.0 # type: googlecloudstorage
fs.onedatarestfs # type: onedata, depends on onedatafilerestclient
fs.onedatarestfs==21.2.5.1 # type: onedata, depends on onedatafilerestclient
fs-basespace # type: basespace
fs-azureblob # type: azure

Expand Down
2 changes: 1 addition & 1 deletion lib/galaxy/dependencies/dev-requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ myst-parser==3.0.1 ; python_version >= "3.8" and python_version < "3.13"
nh3==0.2.17 ; python_version >= "3.8" and python_version < "3.13"
numpy==1.24.4 ; python_version >= "3.8" and python_version < "3.9"
numpy==1.26.4 ; python_version >= "3.9" and python_version < "3.13"
onedatafilerestclient==21.2.5rc1 ; python_version >= "3.8" and python_version < "3.13"
onedatafilerestclient==21.2.5.1 ; python_version >= "3.8" and python_version < "3.13"
outcome==1.3.0.post0 ; python_version >= "3.8" and python_version < "3.13"
packaging==24.0 ; python_version >= "3.8" and python_version < "3.13"
pathspec==0.12.1 ; python_version >= "3.8" and python_version < "3.13"
Expand Down
2 changes: 1 addition & 1 deletion lib/galaxy/files/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -476,4 +476,4 @@ def file_sources(self):

@property
def anonymous(self) -> bool:
return bool(self._kwd.get("username"))
return not bool(self._kwd.get("username"))
17 changes: 1 addition & 16 deletions lib/galaxy/files/sources/_pyfilesystem2.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,22 +85,7 @@ def _list(
raise MessageException(f"Problem listing file source path {path}. Reason: {e}") from e

def _get_total_matches_count(self, fs: FS, path: str, filter: Optional[List[str]] = None) -> int:
# For some reason, using "*" as glob does not return all files and directories, only files.
# So we need to count files and directories "*/" separately.
# Also, some filesystems do not properly support directories count (like Google Cloud Storage),
# so we need to catch TypeError exceptions and fallback to 0.
files_glob_pattern = f"{path}/{filter[0] if filter else '*'}"
try:
files_count = fs.glob(files_glob_pattern).count().files
except TypeError:
files_count = 0

directory_glob_pattern = f"{files_glob_pattern}/"
try:
directories_count = fs.glob(directory_glob_pattern).count().directories
except TypeError:
directories_count = 0
return files_count + directories_count
return sum(1 for _ in fs.filterdir(path, namespaces=["basic"], files=filter, dirs=filter))

def _to_page(self, limit: Optional[int] = None, offset: Optional[int] = None) -> Optional[Tuple[int, int]]:
if limit is None and offset is None:
Expand Down
12 changes: 10 additions & 2 deletions lib/galaxy/job_execution/output_collect.py
Original file line number Diff line number Diff line change
Expand Up @@ -521,7 +521,8 @@ def collect_primary_datasets(job_context: Union[JobContext, SessionlessJobContex
outdata.designation = designation
outdata.dataset.external_filename = None # resets filename_override
# Move data from temp location to dataset location
job_context.object_store.update_from_file(outdata.dataset, file_name=filename, create=True)
if not outdata.dataset.purged:
job_context.object_store.update_from_file(outdata.dataset, file_name=filename, create=True)
primary_output_assigned = True
continue
if name not in primary_datasets:
Expand Down Expand Up @@ -554,6 +555,7 @@ def collect_primary_datasets(job_context: Union[JobContext, SessionlessJobContex
dataset_attributes=new_primary_datasets_attributes,
creating_job_id=job_context.get_job_id() if job_context else None,
storage_callbacks=storage_callbacks,
purged=outdata.dataset.purged,
)
# Associate new dataset with job
job_context.add_output_dataset_association(f"__new_primary_file_{name}|{designation}__", primary_data)
Expand All @@ -563,7 +565,13 @@ def collect_primary_datasets(job_context: Union[JobContext, SessionlessJobContex
if primary_output_assigned:
outdata.name = new_outdata_name
outdata.init_meta()
outdata.set_meta()
if not outdata.dataset.purged:
try:
outdata.set_meta()
except Exception:
# We don't want to fail here on a single "bad" discovered dataset
log.debug("set meta failed for %s", outdata, exc_info=True)
outdata.state = HistoryDatasetAssociation.states.FAILED_METADATA
outdata.set_peek()
outdata.discovered = True
sa_session = job_context.sa_session
Expand Down
10 changes: 7 additions & 3 deletions lib/galaxy/jobs/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2004,10 +2004,14 @@ def fail(message=job.info, exception=None):
quota_source_info = None
# Once datasets are collected, set the total dataset size (includes extra files)
for dataset_assoc in job.output_datasets:
if not dataset_assoc.dataset.dataset.purged:
dataset = dataset_assoc.dataset.dataset
if not dataset.purged:
# assume all datasets in a job get written to the same objectstore
quota_source_info = dataset_assoc.dataset.dataset.quota_source_info
collected_bytes += dataset_assoc.dataset.set_total_size()
quota_source_info = dataset.quota_source_info
collected_bytes += dataset.set_total_size()
else:
# Purge, in case job wrote directly to object store
dataset.full_delete()

user = job.user
if user and collected_bytes > 0 and quota_source_info is not None and quota_source_info.use:
Expand Down
6 changes: 5 additions & 1 deletion lib/galaxy/jobs/command_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,11 @@ def __copy_if_exists_command(work_dir_output):
source_file, destination = work_dir_output
if "?" in source_file or "*" in source_file:
source_file = source_file.replace("*", '"*"').replace("?", '"?"')
return f'\nif [ -f "{source_file}" ] ; then cp "{source_file}" "{destination}" ; fi'
# Check if source and destination exist.
# Users can purge outputs before the job completes,
# in that case we don't want to copy the output to a purged path.
# Static, non work_dir_output files are handled in job_finish code.
return f'\nif [ -f "{source_file}" -a -f "{destination}" ] ; then cp "{source_file}" "{destination}" ; fi'


class CommandsBuilder:
Expand Down
Loading
Loading