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

feat: return mutation as object that does not need to be reparsed #357

Merged
merged 1 commit into from
Mar 26, 2024

Conversation

Taepper
Copy link
Collaborator

@Taepper Taepper commented Mar 23, 2024

@fengelniederhammer
Copy link
Contributor

I was about to bring up that discussion anyway, I really like the change 👍

In LAPIS we will most probably keep the old mutation field for backwards compatibility and convenience. Do we want to do the same here? Or do we rather want to strip the redundant data from the SILO responses?

@Taepper
Copy link
Collaborator Author

Taepper commented Mar 25, 2024

That is a good point, just returning both might be viable. What do you think @chaoran-chen. Keeping both fields only in LAPIS or also in SILO?

@fengelniederhammer
Copy link
Contributor

(Also just to keep in mind: this PR should not be merged without preparation in LAPIS if it still contains breaking changes.)

@Taepper
Copy link
Collaborator Author

Taepper commented Mar 25, 2024

Similar to GenSpectrum/LAPIS#710 it might be better to include both in SILO, then we can order by all fields that are also returned by LAPIS

@Taepper Taepper force-pushed the mutation-destructured branch from 8df6f1d to a9c88b8 Compare March 25, 2024 13:14
@Taepper
Copy link
Collaborator Author

Taepper commented Mar 25, 2024

Should be ready to merge now, as it is non-breaking for LAPIS. (auto-merge will act after tests passed and approving review)

@Taepper Taepper enabled auto-merge (squash) March 25, 2024 13:15
@Taepper
Copy link
Collaborator Author

Taepper commented Mar 25, 2024

Some QOL changes introduced to query.test.js, where the actual result is directly written to a file that can be easier compared and copied

@Taepper Taepper disabled auto-merge March 25, 2024 15:04
@Taepper Taepper enabled auto-merge (squash) March 25, 2024 15:04
@Taepper Taepper requested a review from pflanze March 25, 2024 15:04
@Taepper Taepper disabled auto-merge March 25, 2024 15:48
@Taepper Taepper merged commit a93abbf into main Mar 26, 2024
6 checks passed
@Taepper Taepper deleted the mutation-destructured branch March 26, 2024 12:18
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.

2 participants