Skip to content

feat(lapis): also handle SILO returning arrow date values#1700

Merged
fengelniederhammer merged 3 commits into
mainfrom
handle-silo-date
May 27, 2026
Merged

feat(lapis): also handle SILO returning arrow date values#1700
fengelniederhammer merged 3 commits into
mainfrom
handle-silo-date

Conversation

@Taepper
Copy link
Copy Markdown
Contributor

@Taepper Taepper commented May 21, 2026

For testing, we will first need to handle sending saneql queries, which has been finished in #1682

Now this PR adds handling of returned arrow batches that also contain date arrays

PR Checklist

- [ ] All necessary documentation has been adapted.
- [ ] All necessary changes are explained in the llms.txt.

  • The implemented feature is covered by an appropriate test.

@vercel
Copy link
Copy Markdown

vercel Bot commented May 21, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
lapis Ready Ready Preview, Comment May 27, 2026 6:34am

Request Review

@Taepper Taepper force-pushed the handle-silo-date branch from f047c51 to 86a1be2 Compare May 21, 2026 09:55
@Taepper Taepper changed the base branch from main to 1682-send-silo-queries-as-saneql May 21, 2026 11:34
Base automatically changed from 1682-send-silo-queries-as-saneql to main May 26, 2026 06:13
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the SILO Arrow response handling so LAPIS can correctly serialize Arrow date32/day values into JSON, preventing unexpected stringification of raw day offsets.

Changes:

  • Add explicit handling for DateDayVector (Arrow date32) in fieldValueAsJsonNode to return an ISO-8601 date string.
  • Add the required Arrow vector import (DateDayVector).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown
Contributor

@fengelniederhammer fengelniederhammer left a comment

Choose a reason for hiding this comment

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

What Copilot said. The SiloClientTest tests the deserialization of responses, I think that's were we should also test this change.

Otherwise looks good, the e2e are back to green!

@Taepper Taepper marked this pull request as ready for review May 26, 2026 12:55
Copy link
Copy Markdown
Contributor

@fengelniederhammer fengelniederhammer left a comment

Choose a reason for hiding this comment

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

Thanks! :)

@fengelniederhammer fengelniederhammer merged commit 0d5387c into main May 27, 2026
9 checks passed
@fengelniederhammer fengelniederhammer deleted the handle-silo-date branch May 27, 2026 06:59
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.

3 participants