From d42236e578523d7510587d1b127e6bc8facfeda1 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Fri, 31 May 2024 09:52:55 -0400 Subject: [PATCH] More structured indexing for user data objects. I think I clung to the idea of having two ways to do this too long. I did a bunch of testing with the old way (exposing integer references based on numeric database primary keys). Having a config option that if changed would break everything exisiting is also a symptom of maybe me clinging too hard for too long. The result of dropping this option is a much cleaner API schema, simpler API objects, and better typing throughout. We can also be more certain of how things entering the Vault are stored - I think being more strucutured about this is good. Ultimately, the future facing stuff (e.g. OAuth 2.0) is going to require UUIDs so that I can store things like refresh tokens in the store before the object has been fully created. --- client/src/api/schema/schema.ts | 16 ++--- .../ConfigTemplates/test_fixtures.ts | 1 - .../FileSources/Instances/EditInstance.vue | 4 +- .../FileSources/Instances/EditSecrets.vue | 2 +- .../Instances/InstanceDropdown.vue | 4 +- .../FileSources/Instances/UpgradeForm.vue | 2 +- .../FileSources/Instances/UpgradeInstance.vue | 2 +- .../FileSources/Instances/instance.ts | 2 +- .../FileSources/Instances/services.ts | 2 +- .../ObjectStore/Instances/EditInstance.vue | 4 +- .../ObjectStore/Instances/EditSecrets.vue | 2 +- .../Instances/InstanceDropdown.vue | 4 +- .../ObjectStore/Instances/UpgradeForm.vue | 2 +- .../ObjectStore/Instances/UpgradeInstance.vue | 2 +- .../ObjectStore/Instances/instance.ts | 2 +- .../ObjectStore/Instances/services.ts | 2 +- client/src/stores/fileSourceInstancesStore.ts | 2 +- .../src/stores/objectStoreInstancesStore.ts | 2 +- lib/galaxy/app.py | 1 - lib/galaxy/config/schemas/config_schema.yml | 13 ----- lib/galaxy/managers/file_source_instances.py | 58 +++++-------------- lib/galaxy/managers/object_store_instances.py | 47 +++++---------- lib/galaxy/model/__init__.py | 8 +-- lib/galaxy/objectstore/__init__.py | 16 ++--- .../objectstore/unittest_utils/__init__.py | 1 - lib/galaxy/webapps/galaxy/api/file_sources.py | 2 +- lib/galaxy/webapps/galaxy/api/object_store.py | 4 +- 27 files changed, 64 insertions(+), 143 deletions(-) diff --git a/client/src/api/schema/schema.ts b/client/src/api/schema/schema.ts index 07b01dd55e13..e35e5d73c7d9 100644 --- a/client/src/api/schema/schema.ts +++ b/client/src/api/schema/schema.ts @@ -12651,8 +12651,6 @@ export interface components { device?: string | null; /** Hidden */ hidden: boolean; - /** Id */ - id: number | string; /** Name */ name?: string | null; /** Object Store Id */ @@ -12730,8 +12728,6 @@ export interface components { description: string | null; /** Hidden */ hidden: boolean; - /** Id */ - id: string | number; /** Name */ name: string; /** Purged */ @@ -14972,7 +14968,7 @@ export interface operations { header?: { "run-as"?: string | null; }; - /** @description The index for a persisted UserFileSourceStore object. */ + /** @description The UUID index for a persisted UserFileSourceStore object. */ path: { user_file_source_id: string; }; @@ -14999,7 +14995,7 @@ export interface operations { header?: { "run-as"?: string | null; }; - /** @description The index for a persisted UserFileSourceStore object. */ + /** @description The UUID index for a persisted UserFileSourceStore object. */ path: { user_file_source_id: string; }; @@ -15034,7 +15030,7 @@ export interface operations { header?: { "run-as"?: string | null; }; - /** @description The index for a persisted UserFileSourceStore object. */ + /** @description The UUID index for a persisted UserFileSourceStore object. */ path: { user_file_source_id: string; }; @@ -20896,7 +20892,7 @@ export interface operations { header?: { "run-as"?: string | null; }; - /** @description The identifier used to index a persisted UserObjectStore object. */ + /** @description The UUID used to indentify a persisted UserObjectStore object. */ path: { user_object_store_id: string; }; @@ -20923,7 +20919,7 @@ export interface operations { header?: { "run-as"?: string | null; }; - /** @description The identifier used to index a persisted UserObjectStore object. */ + /** @description The UUID used to indentify a persisted UserObjectStore object. */ path: { user_object_store_id: string; }; @@ -20958,7 +20954,7 @@ export interface operations { header?: { "run-as"?: string | null; }; - /** @description The identifier used to index a persisted UserObjectStore object. */ + /** @description The UUID used to indentify a persisted UserObjectStore object. */ path: { user_object_store_id: string; }; diff --git a/client/src/components/ConfigTemplates/test_fixtures.ts b/client/src/components/ConfigTemplates/test_fixtures.ts index 9e90afa3396c..f29e8d5bd598 100644 --- a/client/src/components/ConfigTemplates/test_fixtures.ts +++ b/client/src/components/ConfigTemplates/test_fixtures.ts @@ -114,7 +114,6 @@ export const OBJECT_STORE_INSTANCE: UserConcreteObjectStore = { secrets: ["oldsecret", "droppedsecret"], quota: { enabled: false }, private: false, - id: 4, uuid: "112f889f-72d7-4619-a8e8-510a8c685aa7", active: true, hidden: false, diff --git a/client/src/components/FileSources/Instances/EditInstance.vue b/client/src/components/FileSources/Instances/EditInstance.vue index fbea25de628c..40e62a309da1 100644 --- a/client/src/components/FileSources/Instances/EditInstance.vue +++ b/client/src/components/FileSources/Instances/EditInstance.vue @@ -13,7 +13,7 @@ import EditSecrets from "./EditSecrets.vue"; import InstanceForm from "@/components/ConfigTemplates/InstanceForm.vue"; interface Props { - instanceId: number | string; + instanceId: string; } const props = defineProps(); @@ -36,7 +36,7 @@ const loadingMessage = "Loading file source template and instance information"; async function onSubmit(formData: any) { if (template.value) { const payload = editFormDataToPayload(template.value, formData); - const args = { user_file_source_id: String(instance?.value?.id) }; + const args = { user_file_source_id: String(instance?.value?.uuid) }; const { data: fileSource } = await update({ ...args, ...payload }); await onUpdate(fileSource); } diff --git a/client/src/components/FileSources/Instances/EditSecrets.vue b/client/src/components/FileSources/Instances/EditSecrets.vue index dd14126a1242..200bcd85011c 100644 --- a/client/src/components/FileSources/Instances/EditSecrets.vue +++ b/client/src/components/FileSources/Instances/EditSecrets.vue @@ -19,7 +19,7 @@ async function onUpdate(secretName: string, secretValue: string) { secret_name: secretName, secret_value: secretValue, }; - const args = { user_file_source_id: String(props.fileSource.id) }; + const args = { user_file_source_id: String(props.fileSource.uuid) }; await update({ ...args, ...payload }); } diff --git a/client/src/components/FileSources/Instances/InstanceDropdown.vue b/client/src/components/FileSources/Instances/InstanceDropdown.vue index 75b93ec5329e..9762fdc21dcc 100644 --- a/client/src/components/FileSources/Instances/InstanceDropdown.vue +++ b/client/src/components/FileSources/Instances/InstanceDropdown.vue @@ -15,8 +15,8 @@ interface Props { } const props = defineProps(); -const routeEdit = computed(() => `/file_source_instances/${props.fileSource.id}/edit`); -const routeUpgrade = computed(() => `/file_source_instances/${props.fileSource.id}/upgrade`); +const routeEdit = computed(() => `/file_source_instances/${props.fileSource.uuid}/edit`); +const routeUpgrade = computed(() => `/file_source_instances/${props.fileSource.uuid}/upgrade`); const isUpgradable = computed(() => fileSourceTemplatesStore.canUpgrade(props.fileSource.template_id, props.fileSource.template_version) ); diff --git a/client/src/components/FileSources/Instances/UpgradeForm.vue b/client/src/components/FileSources/Instances/UpgradeForm.vue index cf5621e01b24..9f0efa9a9a28 100644 --- a/client/src/components/FileSources/Instances/UpgradeForm.vue +++ b/client/src/components/FileSources/Instances/UpgradeForm.vue @@ -31,7 +31,7 @@ const loadingMessage = "Loading file source template and instance information"; async function onSubmit(formData: any) { const payload = upgradeFormDataToPayload(props.latestTemplate, formData); - const args = { user_file_source_id: String(props.instance.id) }; + const args = { user_file_source_id: String(props.instance.uuid) }; try { const { data: fileSource } = await update({ ...args, ...payload }); await onUpgrade(fileSource); diff --git a/client/src/components/FileSources/Instances/UpgradeInstance.vue b/client/src/components/FileSources/Instances/UpgradeInstance.vue index a2aa1ed672f4..fcd0acebcb08 100644 --- a/client/src/components/FileSources/Instances/UpgradeInstance.vue +++ b/client/src/components/FileSources/Instances/UpgradeInstance.vue @@ -9,7 +9,7 @@ import UpgradeForm from "./UpgradeForm.vue"; import LoadingSpan from "@/components/LoadingSpan.vue"; interface Props { - instanceId: number | string; + instanceId: string; } const props = defineProps(); diff --git a/client/src/components/FileSources/Instances/instance.ts b/client/src/components/FileSources/Instances/instance.ts index f45fd6bf76dd..26755846ae53 100644 --- a/client/src/components/FileSources/Instances/instance.ts +++ b/client/src/components/FileSources/Instances/instance.ts @@ -4,7 +4,7 @@ import type { FileSourceTemplateSummary, UserFileSourceModel } from "@/api/fileS import { useFileSourceInstancesStore } from "@/stores/fileSourceInstancesStore"; import { useFileSourceTemplatesStore } from "@/stores/fileSourceTemplatesStore"; -export function useInstanceAndTemplate(instanceIdRef: Ref) { +export function useInstanceAndTemplate(instanceIdRef: Ref) { const fileSourceTemplatesStore = useFileSourceTemplatesStore(); const fileSourceInstancesStore = useFileSourceInstancesStore(); fileSourceInstancesStore.fetchInstances(); diff --git a/client/src/components/FileSources/Instances/services.ts b/client/src/components/FileSources/Instances/services.ts index 8f3f33243e2c..357e792f9f22 100644 --- a/client/src/components/FileSources/Instances/services.ts +++ b/client/src/components/FileSources/Instances/services.ts @@ -7,7 +7,7 @@ export const update = fetcher.path("/api/file_source_instances/{user_file_source export async function hide(instance: UserFileSourceModel) { const payload = { hidden: true }; - const args = { user_file_source_id: String(instance?.id) }; + const args = { user_file_source_id: String(instance?.uuid) }; const { data: fileSource } = await update({ ...args, ...payload }); return fileSource; } diff --git a/client/src/components/ObjectStore/Instances/EditInstance.vue b/client/src/components/ObjectStore/Instances/EditInstance.vue index ab33625d867b..0c6ea67cf4a6 100644 --- a/client/src/components/ObjectStore/Instances/EditInstance.vue +++ b/client/src/components/ObjectStore/Instances/EditInstance.vue @@ -13,7 +13,7 @@ import EditSecrets from "./EditSecrets.vue"; import InstanceForm from "@/components/ConfigTemplates/InstanceForm.vue"; interface Props { - instanceId: number | string; + instanceId: string; } const props = defineProps(); @@ -36,7 +36,7 @@ const loadingMessage = "Loading storage location template and instance informati async function onSubmit(formData: any) { if (template.value) { const payload = editFormDataToPayload(template.value, formData); - const args = { user_object_store_id: String(instance?.value?.id) }; + const args = { user_object_store_id: String(instance?.value?.uuid) }; const { data: objectStore } = await update({ ...args, ...payload }); await onUpdate(objectStore); } diff --git a/client/src/components/ObjectStore/Instances/EditSecrets.vue b/client/src/components/ObjectStore/Instances/EditSecrets.vue index 56c1e7cd9641..f300779adc11 100644 --- a/client/src/components/ObjectStore/Instances/EditSecrets.vue +++ b/client/src/components/ObjectStore/Instances/EditSecrets.vue @@ -20,7 +20,7 @@ async function onUpdate(secretName: string, secretValue: string) { secret_name: secretName, secret_value: secretValue, }; - const args = { user_object_store_id: String(props.objectStore.id) }; + const args = { user_object_store_id: String(props.objectStore.uuid) }; await update({ ...args, ...payload }); } diff --git a/client/src/components/ObjectStore/Instances/InstanceDropdown.vue b/client/src/components/ObjectStore/Instances/InstanceDropdown.vue index 00ef24e61eaf..f00bde90b603 100644 --- a/client/src/components/ObjectStore/Instances/InstanceDropdown.vue +++ b/client/src/components/ObjectStore/Instances/InstanceDropdown.vue @@ -15,8 +15,8 @@ interface Props { } const props = defineProps(); -const routeEdit = computed(() => `/object_store_instances/${props.objectStore.id}/edit`); -const routeUpgrade = computed(() => `/object_store_instances/${props.objectStore.id}/upgrade`); +const routeEdit = computed(() => `/object_store_instances/${props.objectStore.uuid}/edit`); +const routeUpgrade = computed(() => `/object_store_instances/${props.objectStore.uuid}/upgrade`); const isUpgradable = computed(() => objectStoreTemplatesStore.canUpgrade(props.objectStore.template_id, props.objectStore.template_version) ); diff --git a/client/src/components/ObjectStore/Instances/UpgradeForm.vue b/client/src/components/ObjectStore/Instances/UpgradeForm.vue index 8bd3917a6072..28aa658f3106 100644 --- a/client/src/components/ObjectStore/Instances/UpgradeForm.vue +++ b/client/src/components/ObjectStore/Instances/UpgradeForm.vue @@ -32,7 +32,7 @@ const loadingMessage = "Loading storage location template and instance informati async function onSubmit(formData: any) { const payload = upgradeFormDataToPayload(props.latestTemplate, formData); - const args = { user_object_store_id: String(props.instance.id) }; + const args = { user_object_store_id: String(props.instance.uuid) }; try { const { data: objectStore } = await update({ ...args, ...payload }); await onUpgrade(objectStore); diff --git a/client/src/components/ObjectStore/Instances/UpgradeInstance.vue b/client/src/components/ObjectStore/Instances/UpgradeInstance.vue index b9038c0a3082..e4a8cc65b122 100644 --- a/client/src/components/ObjectStore/Instances/UpgradeInstance.vue +++ b/client/src/components/ObjectStore/Instances/UpgradeInstance.vue @@ -9,7 +9,7 @@ import UpgradeForm from "./UpgradeForm.vue"; import LoadingSpan from "@/components/LoadingSpan.vue"; interface Props { - instanceId: number | string; + instanceId: string; } const props = defineProps(); diff --git a/client/src/components/ObjectStore/Instances/instance.ts b/client/src/components/ObjectStore/Instances/instance.ts index 7ab30bfda7d7..74d838319562 100644 --- a/client/src/components/ObjectStore/Instances/instance.ts +++ b/client/src/components/ObjectStore/Instances/instance.ts @@ -6,7 +6,7 @@ import { useObjectStoreTemplatesStore } from "@/stores/objectStoreTemplatesStore import type { UserConcreteObjectStore } from "./types"; -export function useInstanceAndTemplate(instanceIdRef: Ref) { +export function useInstanceAndTemplate(instanceIdRef: Ref) { const objectStoreTemplatesStore = useObjectStoreTemplatesStore(); const objectStoreInstancesStore = useObjectStoreInstancesStore(); objectStoreInstancesStore.fetchInstances(); diff --git a/client/src/components/ObjectStore/Instances/services.ts b/client/src/components/ObjectStore/Instances/services.ts index cf2cfc9aacfa..abb7bb6781c7 100644 --- a/client/src/components/ObjectStore/Instances/services.ts +++ b/client/src/components/ObjectStore/Instances/services.ts @@ -8,7 +8,7 @@ export const update = fetcher.path("/api/object_store_instances/{user_object_sto export async function hide(instance: UserConcreteObjectStore) { const payload = { hidden: true }; - const args = { user_object_store_id: String(instance?.id) }; + const args = { user_object_store_id: String(instance?.uuid) }; const { data: objectStore } = await update({ ...args, ...payload }); return objectStore; } diff --git a/client/src/stores/fileSourceInstancesStore.ts b/client/src/stores/fileSourceInstancesStore.ts index 5b2bcf97b821..ec31d00e1e21 100644 --- a/client/src/stores/fileSourceInstancesStore.ts +++ b/client/src/stores/fileSourceInstancesStore.ts @@ -22,7 +22,7 @@ export const useFileSourceInstancesStore = defineStore("fileSourceInstances", { return !state.fetched; }, getInstance: (state) => { - return (id: number | string) => state.instances.find((i) => i.id.toString() == id.toString()); + return (uuid: string) => state.instances.find((i) => i.uuid == uuid); }, }, actions: { diff --git a/client/src/stores/objectStoreInstancesStore.ts b/client/src/stores/objectStoreInstancesStore.ts index b961ba63ea2b..e825c9c63e86 100644 --- a/client/src/stores/objectStoreInstancesStore.ts +++ b/client/src/stores/objectStoreInstancesStore.ts @@ -22,7 +22,7 @@ export const useObjectStoreInstancesStore = defineStore("objectStoreInstances", return !state.fetched; }, getInstance: (state) => { - return (id: number | string) => state.instances.find((i) => i.id.toString() == id.toString()); + return (uuid: string) => state.instances.find((i) => i.uuid == uuid); }, }, actions: { diff --git a/lib/galaxy/app.py b/lib/galaxy/app.py index 1d65289f499a..0c69d1f57279 100644 --- a/lib/galaxy/app.py +++ b/lib/galaxy/app.py @@ -434,7 +434,6 @@ def _configure_object_store(self, **kwds): gid=self.config.gid, object_store_cache_size=self.config.object_store_cache_size, object_store_cache_path=self.config.object_store_cache_path, - user_config_templates_index_by=self.config.user_config_templates_index_by, user_config_templates_use_saved_configuration=self.config.user_config_templates_use_saved_configuration, ) self._register_singleton(UserObjectStoresAppConfig, app_config) diff --git a/lib/galaxy/config/schemas/config_schema.yml b/lib/galaxy/config/schemas/config_schema.yml index 1fb105a22652..e18dc2aa28e8 100644 --- a/lib/galaxy/config/schemas/config_schema.yml +++ b/lib/galaxy/config/schemas/config_schema.yml @@ -581,19 +581,6 @@ mapping: desc: | Configured user file source templates embedded into Galaxy's config. - user_config_templates_index_by: - type: str - default: 'uuid' - required: false - enum: ['uuid', 'id'] - desc: | - Configure URIs for user object stores to use either the object ID ('id') - or UUIDs ('uuid'). Either is fine really, Galaxy doesn't typically expose - database objects by 'id' but there isn't any obvious disadvantage to doing - it in this case and it keeps user exposed URIs much smaller. The default of - UUID feels a little more like a typical way to do this within Galaxy though. - Do not change this value once user object stores have been created. - user_config_templates_use_saved_configuration: type: str default: 'fallback' diff --git a/lib/galaxy/managers/file_source_instances.py b/lib/galaxy/managers/file_source_instances.py index ba3bacde540e..120329188138 100644 --- a/lib/galaxy/managers/file_source_instances.py +++ b/lib/galaxy/managers/file_source_instances.py @@ -8,7 +8,6 @@ Optional, Set, Tuple, - Union, ) from uuid import uuid4 @@ -88,7 +87,6 @@ class UserFileSourceModel(BaseModel): - id: Union[str, int] uuid: str uri_root: str name: str @@ -104,17 +102,13 @@ class UserFileSourceModel(BaseModel): class UserDefinedFileSourcesConfig(BaseModel): - user_config_templates_index_by: Literal["uuid", "id"] user_config_templates_use_saved_configuration: Literal["fallback", "preferred", "never"] @staticmethod def from_app_config(config) -> "UserDefinedFileSourcesConfig": - user_config_templates_index_by = config.user_config_templates_index_by - assert user_config_templates_index_by in ["uuid", "id"] user_config_templates_use_saved_configuration = config.user_config_templates_use_saved_configuration assert user_config_templates_use_saved_configuration in ["fallback", "preferred", "never"] return UserDefinedFileSourcesConfig( - user_config_templates_index_by=user_config_templates_index_by, user_config_templates_use_saved_configuration=user_config_templates_use_saved_configuration, ) @@ -148,16 +142,16 @@ def index(self, trans: ProvidesUserContext) -> List[UserFileSourceModel]: stores = self._sa_session.query(UserFileSource).filter(UserFileSource.user_id == trans.user.id).all() return [self._to_model(trans, s) for s in stores] - def show(self, trans: ProvidesUserContext, id: Union[str, int]) -> UserFileSourceModel: - user_file_source = self._get(trans, id) + def show(self, trans: ProvidesUserContext, uuid: str) -> UserFileSourceModel: + user_file_source = self._get(trans, uuid) return self._to_model(trans, user_file_source) - def purge_instance(self, trans: ProvidesUserContext, id: Union[str, int]) -> None: - persisted_file_source = self._get(trans, id) + def purge_instance(self, trans: ProvidesUserContext, uuid: str) -> None: + persisted_file_source = self._get(trans, uuid) purge_template_instance(trans, persisted_file_source, self._app_config) def modify_instance( - self, trans: ProvidesUserContext, id: Union[str, int], payload: ModifyInstancePayload + self, trans: ProvidesUserContext, id: str, payload: ModifyInstancePayload ) -> UserFileSourceModel: if isinstance(payload, UpgradeInstancePayload): return self._upgrade_instance(trans, id, payload) @@ -168,7 +162,7 @@ def modify_instance( return self._update_instance(trans, id, payload) def _upgrade_instance( - self, trans: ProvidesUserContext, id: Union[str, int], payload: UpgradeInstancePayload + self, trans: ProvidesUserContext, id: str, payload: UpgradeInstancePayload ) -> UserFileSourceModel: persisted_file_source = self._get(trans, id) template = self._get_template(persisted_file_source, payload.template_version) @@ -187,7 +181,7 @@ def _upgrade_instance( return self._to_model(trans, persisted_file_source) def _update_instance( - self, trans: ProvidesUserContext, id: Union[str, int], payload: UpdateInstancePayload + self, trans: ProvidesUserContext, id: str, payload: UpdateInstancePayload ) -> UserFileSourceModel: persisted_file_source = self._get(trans, id) template = self._get_template(persisted_file_source) @@ -195,7 +189,7 @@ def _update_instance( return self._to_model(trans, persisted_file_source) def _update_instance_secret( - self, trans: ProvidesUserContext, id: Union[str, int], payload: UpdateInstanceSecretPayload + self, trans: ProvidesUserContext, id: str, payload: UpdateInstanceSecretPayload ) -> UserFileSourceModel: persisted_file_source = self._get(trans, id) template = self._get_template(persisted_file_source) @@ -306,19 +300,11 @@ def _connection_status( exception = e return file_source, connection_exception_to_status("file source", exception) - def _index_filter(self, id: Union[str, int]): - index_by = self._app_config.user_config_templates_index_by - index_filter: Any - if index_by == "id": - id_as_int = int(id) - index_filter = UserFileSource.__table__.c.id == id_as_int - else: - id_as_str = str(id) - index_filter = UserFileSource.__table__.c.uuid == id_as_str - return index_filter + def _index_filter(self, uuid: str): + return UserFileSource.__table__.c.uuid == uuid - def _get(self, trans: ProvidesUserContext, id: Union[str, int]) -> UserFileSource: - filter = self._index_filter(id) + def _get(self, trans: ProvidesUserContext, uuid: str) -> UserFileSource: + filter = self._index_filter(uuid) user_file_source = self._sa_session.query(UserFileSource).filter(filter).one_or_none() if user_file_source is None: raise RequestParameterInvalidException(f"Failed to fetch object store for id {id}") @@ -340,18 +326,10 @@ def _save(self, user_file_source: UserFileSource) -> None: def _to_model(self, trans, persisted_file_source: UserFileSource) -> UserFileSourceModel: file_source_type = persisted_file_source.template.configuration.type secrets = persisted_file_source.template_secrets or [] - index_by = self._app_config.user_config_templates_index_by - response_id: Union[str, int] - if index_by == "id": - ufs_id = str(persisted_file_source.id) - response_id = persisted_file_source.id - else: - ufs_id = str(persisted_file_source.uuid) - response_id = ufs_id + uuid = str(persisted_file_source.uuid) uri_root = f"{USER_FILE_SOURCES_SCHEME}://{ufs_id}" return UserFileSourceModel( - id=response_id, - uuid=str(persisted_file_source.uuid), + uuid=uuid, uri_root=uri_root, type=file_source_type, template_id=persisted_file_source.template_id, @@ -399,13 +377,7 @@ def _user_file_source(self, uri: str) -> Optional[UserFileSource]: uri_root, _ = uri_rest.split("/", 1) else: uri_root = uri_rest - index_by = self._app_config.user_config_templates_index_by - index_filter: Any - if index_by == "id": - index_filter = UserFileSource.__table__.c.id == uri_root - else: - index_filter = UserFileSource.__table__.c.uuid == uri_root - + index_filter = UserFileSource.__table__.c.uuid == uri_root user_object_store: UserFileSource = self._sa_session.query(UserFileSource).filter(index_filter).one() return user_object_store diff --git a/lib/galaxy/managers/object_store_instances.py b/lib/galaxy/managers/object_store_instances.py index 6e345c6b7c9c..754a749d8622 100644 --- a/lib/galaxy/managers/object_store_instances.py +++ b/lib/galaxy/managers/object_store_instances.py @@ -75,7 +75,6 @@ class UserConcreteObjectStoreModel(ConcreteObjectStoreModel): - id: Union[int, str] uuid: str type: ObjectStoreTemplateType template_id: str @@ -109,7 +108,7 @@ def summaries(self) -> ObjectStoreTemplateSummaries: return self._catalog.summaries def modify_instance( - self, trans: ProvidesUserContext, id: Union[str, int], payload: ModifyInstancePayload + self, trans: ProvidesUserContext, id: str, payload: ModifyInstancePayload ) -> UserConcreteObjectStoreModel: if isinstance(payload, UpgradeInstancePayload): return self._upgrade_instance(trans, id, payload) @@ -119,12 +118,12 @@ def modify_instance( assert isinstance(payload, UpdateInstancePayload) return self._update_instance(trans, id, payload) - def purge_instance(self, trans: ProvidesUserContext, id: Union[str, int]) -> None: + def purge_instance(self, trans: ProvidesUserContext, id: str) -> None: persisted_object_store = self._get(trans, id) purge_template_instance(trans, persisted_object_store, self._app_config) def _upgrade_instance( - self, trans: ProvidesUserContext, id: Union[str, int], payload: UpgradeInstancePayload + self, trans: ProvidesUserContext, id: str, payload: UpgradeInstancePayload ) -> UserConcreteObjectStoreModel: persisted_object_store = self._get(trans, id) template = self._get_template(persisted_object_store, payload.template_version) @@ -143,7 +142,7 @@ def _upgrade_instance( return self._to_model(trans, persisted_object_store) def _update_instance( - self, trans: ProvidesUserContext, id: Union[str, int], payload: UpdateInstancePayload + self, trans: ProvidesUserContext, id: str, payload: UpdateInstancePayload ) -> UserConcreteObjectStoreModel: persisted_object_store = self._get(trans, id) template = self._get_template(persisted_object_store) @@ -151,7 +150,7 @@ def _update_instance( return self._to_model(trans, persisted_object_store) def _update_instance_secret( - self, trans: ProvidesUserContext, id: Union[str, int], payload: UpdateInstanceSecretPayload + self, trans: ProvidesUserContext, id: str, payload: UpdateInstanceSecretPayload ) -> UserConcreteObjectStoreModel: persisted_object_store = self._get(trans, id) template = self._get_template(persisted_object_store) @@ -203,14 +202,14 @@ def index(self, trans: ProvidesUserContext) -> List[UserConcreteObjectStoreModel stores = self._sa_session.query(UserObjectStore).filter(UserObjectStore.user_id == trans.user.id).all() return [self._to_model(trans, s) for s in stores] - def show(self, trans: ProvidesUserContext, id: Union[str, int]) -> UserConcreteObjectStoreModel: + def show(self, trans: ProvidesUserContext, id: str) -> UserConcreteObjectStoreModel: user_object_store = self._get(trans, id) return self._to_model(trans, user_object_store) def _save(self, persisted_object_store: UserObjectStore) -> None: save_template_instance(self._sa_session, persisted_object_store) - def _get(self, trans: ProvidesUserContext, id: Union[str, int]) -> UserObjectStore: + def _get(self, trans: ProvidesUserContext, id: str) -> UserObjectStore: filter = self._index_filter(id) user_object_store = self._sa_session.query(UserObjectStore).filter(filter).one_or_none() if user_object_store is None: @@ -277,16 +276,8 @@ def _connection_status( exception = e return object_store, connection_exception_to_status("storage location", exception) - def _index_filter(self, id: Union[str, int]): - index_by = self._app_config.user_config_templates_index_by - index_filter: Any - if index_by == "id": - id_as_int = int(id) - index_filter = UserObjectStore.__table__.c.id == id_as_int - else: - id_as_str = str(id) - index_filter = UserObjectStore.__table__.c.uuid == id_as_str - return index_filter + def _index_filter(self, uuid: str): + return UserObjectStore.__table__.c.uuid == uuid def _get_template( self, persisted_object_store: UserObjectStore, template_version: Optional[int] = None @@ -308,19 +299,12 @@ def _to_model(self, trans, persisted_object_store: UserObjectStore) -> UserConcr object_store_type in ["azure_blob", "s3"], ) secrets = persisted_object_store.template_secrets or [] - uos_id: str - response_id: Union[int, str] - if self._app_config.user_config_templates_index_by == "id": - uos_id = str(persisted_object_store.id) - response_id = persisted_object_store.id - else: - uos_id = str(persisted_object_store.uuid) - response_id = uos_id + uuid = str(persisted_object_store.uuid) object_store_id = f"user_objects://{uos_id}" return UserConcreteObjectStoreModel( - id=response_id, - uuid=str(persisted_object_store.uuid), + id=uuid, + uuid=uuid, type=object_store_type, template_id=persisted_object_store.template_id, template_version=persisted_object_store.template_version, @@ -353,12 +337,7 @@ def __init__( def resolve_object_store_uri_config(self, uri: str) -> ObjectStoreConfiguration: user_object_store_id = uri.split("://", 1)[1] - index_by = self._app_config.user_config_templates_index_by - index_filter: Any - if index_by == "id": - index_filter = UserObjectStore.__table__.c.id == user_object_store_id - else: - index_filter = UserObjectStore.__table__.c.uuid == user_object_store_id + index_filter = UserObjectStore.__table__.c.uuid == user_object_store_id user_object_store: UserObjectStore = self._sa_session.query(UserObjectStore).filter(index_filter).one() secrets = recover_secrets(user_object_store, self._vault, self._app_config) environment = prepare_environment(user_object_store, self._vault, self._app_config) diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index dfe776d2da5c..d04f1b1c1bc4 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -10939,7 +10939,8 @@ def __init__(self, name=None, value=None): class UsesTemplatesAppConfig(Protocol): - user_config_templates_index_by: Literal["uuid", "id"] + # TODO: delete this config protocol def and uses in dev + pass class HasConfigSecrets(RepresentById): @@ -10949,10 +10950,7 @@ class HasConfigSecrets(RepresentById): user: Mapped["User"] def vault_id_prefix(self, app_config: UsesTemplatesAppConfig) -> str: - if app_config.user_config_templates_index_by == "id": - id_str = str(self.id) - else: - id_str = str(self.uuid) + id_str = str(self.uuid) user_vault_id_prefix = f"{self.secret_config_type}/{id_str}" return user_vault_id_prefix diff --git a/lib/galaxy/objectstore/__init__.py b/lib/galaxy/objectstore/__init__.py index 44d11ae5c6c3..1e7ea0afdcc8 100644 --- a/lib/galaxy/objectstore/__init__.py +++ b/lib/galaxy/objectstore/__init__.py @@ -1364,17 +1364,10 @@ def validate_selected_object_store_id(self, user, object_store_id: Optional[str] if not user: return "Supplied object store id is not accessible" rest_of_uri = object_store_id.split("://", 1)[1] - index_by = self.config.user_config_templates_index_by - if index_by == "id": - user_object_store_id = int(rest_of_uri) - for user_object_store in user.object_stores: - if user_object_store.id == user_object_store_id: - return None - else: - user_object_store_uuid = rest_of_uri - for user_object_store in user.object_stores: - if str(user_object_store.uuid) == user_object_store_uuid: - return None + user_object_store_uuid = rest_of_uri + for user_object_store in user.object_stores: + if str(user_object_store.uuid) == user_object_store_uuid: + return None return "Supplied object store id was not found" if object_store_id not in self.object_store_ids_allowing_selection(): return "Supplied object store id is not an allowed object store selection" @@ -1658,7 +1651,6 @@ def build_object_store_from_config( class UserObjectStoresAppConfig(BaseModel): object_store_cache_path: str object_store_cache_size: int - user_config_templates_index_by: Literal["uuid", "id"] user_config_templates_use_saved_configuration: Literal["fallback", "preferred", "never"] jobs_directory: str new_file_path: str diff --git a/lib/galaxy/objectstore/unittest_utils/__init__.py b/lib/galaxy/objectstore/unittest_utils/__init__.py index 3de8a1db9db1..9181d38af61a 100644 --- a/lib/galaxy/objectstore/unittest_utils/__init__.py +++ b/lib/galaxy/objectstore/unittest_utils/__init__.py @@ -108,7 +108,6 @@ def app_config(tmpdir) -> objectstore.UserObjectStoresAppConfig: gid=0o077, object_store_cache_path=str(tmpdir / "cache"), object_store_cache_size=1, - user_config_templates_index_by="uuid", user_config_templates_use_saved_configuration="fallback", ) return app_config diff --git a/lib/galaxy/webapps/galaxy/api/file_sources.py b/lib/galaxy/webapps/galaxy/api/file_sources.py index b72571373d7b..ef777b0e3283 100644 --- a/lib/galaxy/webapps/galaxy/api/file_sources.py +++ b/lib/galaxy/webapps/galaxy/api/file_sources.py @@ -29,7 +29,7 @@ UserFileSourceIdPathParam: str = Path( - ..., title="User File Source ID", description="The index for a persisted UserFileSourceStore object." + ..., title="User File Source UUID", description="The UUID index for a persisted UserFileSourceStore object." ) diff --git a/lib/galaxy/webapps/galaxy/api/object_store.py b/lib/galaxy/webapps/galaxy/api/object_store.py index f028c963018e..5a1777d8a73d 100644 --- a/lib/galaxy/webapps/galaxy/api/object_store.py +++ b/lib/galaxy/webapps/galaxy/api/object_store.py @@ -49,8 +49,8 @@ UserObjectStoreIdPathParam: str = Path( ..., - title="User Object Store Identifier", - description="The identifier used to index a persisted UserObjectStore object.", + title="User Object Store UUID", + description="The UUID used to indentify a persisted UserObjectStore object.", ) SelectableQueryParam: bool = Query(