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

Follow up on object store selection PR. #15654

Merged
merged 29 commits into from
Mar 3, 2023

Conversation

jmchilton
Copy link
Member

@jmchilton jmchilton commented Feb 27, 2023

There are a bunch of non-functional changes that would modernize the object store selection PR #14073 - it is a project I would love to delegate but I'm going to work through them until the PR is merged in case that doesn't materialize.

  • Pydantic for object store API.
  • FastAPI for new user APIs
  • Pydanic for new user APIs
  • Redo awkward target DOM stuff on HistorySelect component without breaking popover
  • Migrate Vue components to TypeScript and composition API.
    • SelectObjectStore.vue
    • client/src/components/Dataset/DatasetStorage/ObjectStoreRestrictionSpan.vue
    • client/src/components/History/CurrentHistory/HistoryPreferredStore.vue
    • client/src/components/ObjectStore/ObjectStoreBadge.vue
    • client/src/components/ObjectStore/ObjectStoreBadges.vue
    • client/src/components/ObjectStore/ShowSelectedObjectStore.vue
    • client/src/components/Tool/ToolTargetPreferredObjectStorePopover.vue
    • client/src/components/User/DiskUsage/Quota/ProvidedQuotaSourceUsageBar.vue
    • client/src/components/User/UserPreferredObjectStore.vue
    • client/src/components/Workflow/Run/WorkflowSelectPreferredObjectStore.vue
    • client/src/components/Workflow/Run/WorkflowStorageConfiguration.vue
    • client/src/components/Workflow/Run/WorkflowTargetPreferredObjectStorePopover.vue
  • Migrate new axios and providers to fetches and (probably) pinia.
    • objectstore info
    • quota stuff
  • Bug fix - consistent use of fa-hdd (see commit about why - but the original PR had a mix of fa-hdd and fa-database for objectstore selection).
  • Bug fix - only show icon in ... if selectable object stores are available in config.
    • histories (bug fix)
    • tools / users / workflows (already worked properly in original PR)
  • Fine tuning “preferred” vs ... “selection” nomenclature in the UI
    • Automated default based on object store and job configuration stuff.
    • Allow admin to configure this if dynamic job runners are used.

Bigger projects:

  • Framework for ObjectStore config validation/linting…
  • Connect object store linting into configuration linting.
  • Create a UI for the object store linting.

How to test the changes?

(Select all options that apply)

  • I've included appropriate automated tests.
  • This is a refactoring of components with existing test coverage.
  • Instructions for manual testing are as follows:
    1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

@jmchilton jmchilton force-pushed the object_store_ui_followup branch from bceca92 to c4671c7 Compare February 27, 2023 21:41
@jmchilton jmchilton force-pushed the object_store_ui_followup branch from 01d2fa7 to c40672a Compare March 2, 2023 20:50
I had started by using fa-database everywhere but then I implemented the user selection piece and that icon was already used for custom DB keys and then David implemented the storage management functionality and so that icon couldn't be used in the history either. This I think switches everything else over to the now more consitent fa-hdd.
@jmchilton jmchilton marked this pull request as ready for review March 3, 2023 14:58
@github-actions github-actions bot added this to the 23.1 milestone Mar 3, 2023
@jmchilton
Copy link
Member Author

Not done refining this functionality - but I think these are all atomic changes and fixes some presentational issues to give this functionality a better first impression... and the PR is green 😆 so I'm marking this as ready for review and I'll collect more issues and fixes other places.

@@ -74,11 +75,18 @@

UserIdPathParam: DecodedDatabaseIdField = Path(..., title="User ID", description="The ID of the user to get.")
APIKeyPathParam: str = Path(..., title="API Key", description="The API key of the user.")
FlexibleUserIdPathParam: str = Path(..., title="User ID", description="The ID of the user to get or __current__.")
Copy link
Member

Choose a reason for hiding this comment

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

I think the type should be a union of EncodedDatabaseId and the literal current here.

:history-preferred-object-store-id="historyPreferredObjectStoreId"
:user="user">
</PreferredStorePopover>
<ConfigProvider v-slot="{ config }">
Copy link
Member

@mvdbeek mvdbeek Mar 3, 2023

Choose a reason for hiding this comment

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

Can you use the useConfig(true) composable here please ? The ConfigProvider triggers a new fetch every time.

@jmchilton jmchilton mentioned this pull request Mar 3, 2023
24 tasks
@jmchilton jmchilton merged commit 92270af into galaxyproject:dev Mar 3, 2023
@jmchilton
Copy link
Member Author

More follow up issues are being tracked in #15692.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants