-
Notifications
You must be signed in to change notification settings - Fork 558
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
Improve study-view treatment api's performance #10261
Conversation
...mybatis/src/test/java/org/cbioportal/persistence/mybatis/TreatmentMyBatisRepositoryTest.java
Outdated
Show resolved
Hide resolved
...mybatis/src/test/java/org/cbioportal/persistence/mybatis/TreatmentMyBatisRepositoryTest.java
Outdated
Show resolved
Hide resolved
service/src/main/java/org/cbioportal/service/impl/TreatmentServiceImpl.java
Outdated
Show resolved
Hide resolved
@@ -25,7 +25,7 @@ private Pair<List<String>, List<String>> filterIds(List<String> sampleIds, List< | |||
} | |||
Set<String> studiesWithTreatments = studyIds.stream() | |||
.distinct() | |||
.filter(studyId -> treatmentRepository.studyIdHasTreatments(studyId, key)) | |||
.filter(studyId -> treatmentRepository.hasTreatmentData(Collections.singletonList(studyId), key)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might not be appropriate for this PR but do you think this could be done as a SQL query?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could. Also we might not need this once after moving to new database. I would prefer to leave it as is for now and revisit after moving to new database.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you write a ticket for this. Even with a new database we may still need to address this issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
service/src/main/java/org/cbioportal/service/impl/TreatmentServiceImpl.java
Show resolved
Hide resolved
Kudos, SonarCloud Quality Gate passed! |
I think we can ignore it. It is related to db data init. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
Improve performance for
/treatments/display-patient
/treatments/display-sample
/treatments/sample
/treatments/patient
Improves performance by 10x