Skip to content

Conversation

@aerosol
Copy link
Member

@aerosol aerosol commented Sep 23, 2025

Changes

This lets us have an interface to lookup regular site ids per consolidated view id efficiently. I don't think it needs more tests coverage at this point - since all the caches and their common characteristics are thoroughly tested elsewhere.

Tests

  • Automated tests have been added
  • This PR does not require tests

Changelog

  • Entry has been added to changelog
  • This PR does not make a user-facing change

Documentation

  • Docs have been updated
  • This change does not need a documentation update

Dark mode

  • The UI has been tested both in dark and light mode
  • This PR does not change the UI

@aerosol aerosol marked this pull request as ready for review September 25, 2025 05:52
@aerosol aerosol changed the title wip ConsolidatedView.Cache - first pass Sep 25, 2025
Courtesy by @zoldar

Co-authored-by: Adrian Gruntkowski <adrian.gruntkowski@gmail.com>
@aerosol
Copy link
Member Author

aerosol commented Sep 25, 2025

oof wait this is fun

09:41:57.022 [error] Error refreshing 'consolidated_views' - %Postgrex.Error{message: nil, postgres: %{code: :internal_error, line: "1194", message: "invalid memory alloc request size 1073741824", file: "mcxt.c", unknown: "ERROR", severity: "ERROR", pg_code: "XX000", routine: "repalloc"}, connection_id: 236, query: nil}

@aerosol aerosol added this pull request to the merge queue Sep 25, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 25, 2025
Copy link
Contributor

@RobertJoonas RobertJoonas left a comment

Choose a reason for hiding this comment

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

Looking good! Some questions in line.


@impl true
def get_from_source(consolidated_view_id) do
ConsolidatedView.get(consolidated_view_id)
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be ConsolidatedView.site_ids I think?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, excellent, will update tests

Copy link
Member Author

Choose a reason for hiding this comment

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

@aerosol aerosol added this pull request to the merge queue Sep 25, 2025
Merged via the queue into master with commit 54a66c2 Sep 25, 2025
16 checks passed
@aerosol aerosol deleted the consolidated-cache branch November 24, 2025 12:09
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.

4 participants