@laceysanderson and I had a design dicussion just a bit ago. I felt a bit uneasy about the current design for field actions in PRs #274 and #303 because I built them out of necessity and thought they needed a second look. To help simplify the design and clarify terminology we decided to adjust the current design in the following ways:
Actions
The following field "actions" that appear in the Chado 'plugin_storage_properties' array for a field should be changed in the folloiwng way. Note that every field has a base_table and a chado_table specified.
store: this action is for storing only a single columns in the chado_table specified in the field.
store_id: this is a new action, for indicating that the value is the primary key of the base_table (feature, stock, etc.).
store_link_id: this is a new action for indicating the value is primary key of the chado_table (e.g. analysisprop, analysisfeature, etc.)
store_fixed: this is a new action for indicating the value the same value in a column in the chado_table
join: behaves the same as previously by pulling a single value from a linked table (can be multiple links deep)
replace: same as before
function: same as before
Cache Setting
One other change is that a field could set in the plugin storage properties a setting cache. If TRUE then the value would be saved in the Drupal database tables for the field. By default only the record IDs should be "cached". We felt this was a confusing term, because it's not being "cached". I will rename this to "drupal_store" and it will behave the same. It will also get moved to the property settings instead of the storage plugin settings because it is indepdent of the Chado plugin.
Max Delta
Lastly, to prevent publication of huge numbers of linked values (e.g. analysisfeatures) we will enfoce a "max delta" for publishing. This will prevent the ChadoStorage class from loading or saving more than the max delta values. It will not affect how much data is stored in Chado. The reason is that for large numbers of values, a summary is more useful than lists that contain hundreds of pages for browsing. More design is needed for how to deal with the summary but for now, we just want to create the limit. We will uses a configuration variable to store this for now.
Plan for the Work
Unfortunately, these changes mean that we have to make changes to the ChadoStorage class and both PR #274 and #303 require these changes so the plan is the following:
Remove the functional testing from the PRs that is breaking things. This way we can merge them. We will forgo our requirement for tesing this time.
Merge the PR for the property field because it has been fully tested.
Test the PR for the cvterm field. @laceysanderson will do this. If it passes then she merge and post a new issue of any problems that might still need fixing.
Create a new PR that fixes the ChadoStorage class and implements the change in action as specified above. Here we will require functional testing for all three complex fields (organism, prop, cvterm) before this PR can be merged.
dsenalik
commented
1 year ago
@laceysanderson and I had a design dicussion just a bit ago. I felt a bit uneasy about the current design for field actions in PRs #274 and #303 because I built them out of necessity and thought they needed a second look. To help simplify the design and clarify terminology we decided to adjust the current design in the following ways:
Actions
The following field "actions" that appear in the Chado 'plugin_storage_properties' array for a field should be changed in the folloiwng way. Note that every field has a
base_tableand achado_tablespecified.chado_tablespecified in the field.base_table(feature, stock, etc.).chado_table(e.g. analysisprop, analysisfeature, etc.)Cache Setting
One other change is that a field could set in the plugin storage properties a setting
cache. If TRUE then the value would be saved in the Drupal database tables for the field. By default only the record IDs should be "cached". We felt this was a confusing term, because it's not being "cached". I will rename this to "drupal_store" and it will behave the same. It will also get moved to the property settings instead of the storage plugin settings because it is indepdent of the Chado plugin.Max Delta
Lastly, to prevent publication of huge numbers of linked values (e.g. analysisfeatures) we will enfoce a "max delta" for publishing. This will prevent the ChadoStorage class from loading or saving more than the max delta values. It will not affect how much data is stored in Chado. The reason is that for large numbers of values, a summary is more useful than lists that contain hundreds of pages for browsing. More design is needed for how to deal with the summary but for now, we just want to create the limit. We will uses a configuration variable to store this for now.
Plan for the Work
Unfortunately, these changes mean that we have to make changes to the ChadoStorage class and both PR #274 and #303 require these changes so the plan is the following: