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

Refactor Plate cache #4694

Merged
merged 22 commits into from
Aug 24, 2023
Merged

Refactor Plate cache #4694

merged 22 commits into from
Aug 24, 2023

Conversation

labkey-klum
Copy link
Contributor

@labkey-klum labkey-klum commented Aug 22, 2023

Rationale

Previously, we were caching plates using container as a key. This meant that each time a plate was un-cached, all plates in the container needed to be re-cached. We wanted to move to a finer grained per-plate cache which should improve performance as we anticipate the creation of many plates. One complication that needed to be solved is that Plates can be referenced by:

  • row ID
  • LSID
  • plate name

This version of the cache uses a cache key that can be generated from each of the various reference units. When a plate is added to the cache we populate the cache with the same plate instance using the other 2 cache key variants to avoid having to re-query for a plate we already know about. The same thing happens in reverse when we un-cache a plate. An internal collection is maintained to help with the un-caching of all plates for a container.

Other Changes

  • Moves the app plate experimental feature flag from the biologics module.
  • Add new endpoints to get the domainId of the plate metadata domain and getPlate.api
  • Add a unit test case for setting plate metadata and values
  • Fixed a problem where plate metadata values could not be set using Query.updateRows. This now works however metadata fields are only resolved using their PropertyURI values.

@labkey-klum labkey-klum marked this pull request as ready for review August 22, 2023 23:43
Copy link
Contributor

@labkey-nicka labkey-nicka left a comment

Choose a reason for hiding this comment

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

Nicely refactored!

* capability to resolve vocabulary columns by field key or name.
*/
@Override
protected ColumnInfo resolveColumn(String name)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious what you think it would take to allow us to resolve these by another alias like Properties/columnName or PlateMetadata/columnName? Is that just a whole bag of worms?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, this should be straight forward and I'll add it to this PR as well as update the test case to verify resolving by either PropertyURI or by FieldKey (Properties/columnName)

@labkey-klum labkey-klum merged commit 3448513 into develop Aug 24, 2023
1 of 3 checks passed
@labkey-klum labkey-klum deleted the fb_plate_cache branch August 24, 2023 18:17
@labkey-klum labkey-klum linked an issue Aug 24, 2023 that may be closed by this pull request
15 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AbDisc, Refactor Plate Cache, cache custom props
2 participants