Skip to content

BI-1864 - Experiment Export doesn't consistently work#276

Merged
timparsons merged 1 commit intorelease/0.8from
bug/BI-1864
Jul 28, 2023
Merged

BI-1864 - Experiment Export doesn't consistently work#276
timparsons merged 1 commit intorelease/0.8from
bug/BI-1864

Conversation

@timparsons
Copy link
Member

@timparsons timparsons commented Jul 25, 2023

Description

Story: https://breedinginsight.atlassian.net/browse/BI-1864

Updated the experiment export logic to pull all of the germplasm for a program once at the beginning of the export process rather than hitting the cache for each observation unit being exported. This should improve the performance because the current cache implementation still fetches all germplasm records, then searches in the BrAPI object of interest to filter by the desired criteria. For experiment import, this means the full germplasm list for a program is pulled n times, where n is the number of observation units being exported.

Also added more logging to the export process

Dependencies

bi-web: release/0.8

Testing

  1. Upload an experiment with observations
  2. Navigate to the experiment on the UI
  3. Download the experiment
  4. Ensure that the download works, and that the data in the file matches what was imported in step 1

Checklist:

  • I have performed a self-review of my own code
  • I have tested my code and ensured it meets the acceptance criteria of the story
  • I have tested that my code works with both the brapi-java-server and BreedBase
  • I have create/modified unit tests to cover this change
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to documentation
  • I have run TAF: <please include a link to TAF run>

@timparsons timparsons requested review from a team, dmeidlin and nickpalladino and removed request for a team and dmeidlin July 25, 2023 17:56
@github-actions github-actions bot added the bug Something isn't working label Jul 25, 2023
@timparsons timparsons requested a review from mlm483 July 25, 2023 17:57
mlm483
mlm483 approved these changes Jul 26, 2023
View reviewed changes
Copy link
Contributor

@mlm483 mlm483 left a comment

Choose a reason for hiding this comment

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

This change makes sense to me. It looks OK running against my local bidb and BreedBase, but when I change my .env to point to the rel-test bidb (in order to test the program Alexandra reported), I'm seeing an unending loop of germplasm cache misses and population attemps for the same key when I try to download an experiment. I think rel-test-sugarcanebase is buckling under it's own weight.

I'm OK with merging this change, I think other performance improvements that may impact the speed and reliability (e.g. due to timeouts) of downloads should be separate stories.

@nickpalladino
Copy link
Member

nickpalladino commented Jul 27, 2023

This change makes sense to me. It looks OK running against my local bidb and BreedBase, but when I change my .env to point to the rel-test bidb (in order to test the program Alexandra reported), I'm seeing an unending loop of germplasm cache misses and population attemps for the same key when I try to download an experiment. I think rel-test-sugarcanebase is buckling under it's own weight.

I'm OK with merging this change, I think other performance improvements that may impact the speed and reliability (e.g. due to timeouts) of downloads should be separate stories.

I'll approve but am interested in the follow-up to this if it doesn't appear to fix Alex's issue.

@timparsons timparsons merged commit d7e29b8 into release/0.8 Jul 28, 2023
@timparsons timparsons deleted the bug/BI-1864 branch July 28, 2023 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nickpalladino nickpalladino nickpalladino approved these changes

@mlm483 mlm483 mlm483 approved these changes

Assignees

@nickpalladino nickpalladino

Labels

bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@timparsons @nickpalladino @mlm483