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: adds dashboard census-feature parity across beacon, history, state networks #313

Merged
merged 5 commits into from
Sep 20, 2024

Conversation

mrferris
Copy link
Collaborator

@mrferris mrferris commented Sep 12, 2024

Glados currently only gathers and displays census data for the history network. This PR:

  • turns on state and beacon networks on glados's trin instance
  • runs cartographer instances for state and beacon networks to gather census data
  • adds a selector to the UI that filters and displays census data by network: history, state, or beacon, while filtering via an http param ?network= on the backend.

Bonus: continues the migration toward raw SQL and simplifies the calls for readability and maintenance purposes.

Implements #293 for census data.

Note that all audit data will still show history, which is a bit awkward (history audit data displayed next to beacon/state census data) but it will toggle audit data per network in the next PR.

Screenshot 2024-09-19 at 12 31 08 PM

@mrferris mrferris force-pushed the census branch 5 times, most recently from 57c54d1 to 68b0f7c Compare September 18, 2024 01:55
@mrferris mrferris marked this pull request as ready for review September 18, 2024 03:28
@mrferris mrferris requested review from carver and KolbyML September 19, 2024 13:45
Copy link
Member

@KolbyML KolbyML left a comment

Choose a reason for hiding this comment

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

:shipit: Looks very good. I did find a bug! But it is only a 1 line difference to fix it!

This PR doesn't appear to update audit data depending on the selector, but as I said on discord we can always do that in the next PR, This PR already makes a lot of progress and is very helpful.

glados-web/src/routes.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@carver carver left a comment

Choose a reason for hiding this comment

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

LGTM!

We already talked about this on the call, but I'll just leave it for posterity:

Next time you move a big chunk of code (say >hundred lines) from one place to another, would you mind doing it in a separate refactor commit? I find it much harder to review when there is a move & change at the same time: I can't use the diff to see what changed from the previous implementation. I ended up doing it myself here, which made it so much easier to review the meaningful changes in routes.rs.

glados-web/assets/js/censustimeseries.js Show resolved Hide resolved
migration/src/m20240814_121507_census_subnetwork.rs Outdated Show resolved Hide resolved
glados-web/templates/base.html Show resolved Hide resolved
glados-web/src/routes.rs Show resolved Hide resolved
glados-web/src/routes.rs Show resolved Hide resolved
@mrferris mrferris merged commit 365f7cc into ethereum:master Sep 20, 2024
2 checks passed
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