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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement instance info endpoint (JSON, YAML, TXT) #685

Merged
merged 11 commits into from Jan 30, 2023

Conversation

sigaloid
Copy link
Member

@sigaloid sigaloid commented Jan 3, 2023

https://685.fly.dev/

Fixes #624

I added a feature to the time crate to allow for local timezone loading. I also added serde_yaml so we could return it in YAML form, as well as JSON and regular text; as well as build_html in order to more easily construct an HTML table. These are quite small dependencies. Here's what they look like:

HTML (default with /instance-info)

image

JSON

image

YAML

image

TXT

image

The build.rs is required and standard practice to set a variable - AFAIK it's the easiest way to embed the latest commit hash.

Within instance_info.rs, I made some choices - the InstanceInfo struct will be held in a Lazy. Currently nothing in the struct should change so it's okay to cache it. However, from #624:

  • Maybe some kind of latency statistics? Average latency to reddit, average response latency, etc.
  • Number of responses served

If we add either of these, it will change on the fly, so we need to revisit the use of a Lazy<InstanceInfo>.

Within the instance_info function, I choose html as the default.

In main.rs, I forced evaluation of the two relevant statics (config and instance info) to ensure deploy timestamp is accurate, and to avoid paying the penalty of parsing the config at the first request.

In conclusion, this PR will allow us to implement a system where we can easily check if an instance is actually online and lets us measure versions across the instance ecosystem. If we add metrics like average Reddit latency or number of responses served, we can more accurately predict the performance of certain servers and bias a redirect server (going to work on this in the coming days 馃槈) towards those.

I also added a link to the instance info page into base.html:
image

The groundwork is also laid for a banner #347 - it's been included in the parsing code and is shown on the instance page. We need to discuss whether we want to display it on regular pages (footer? header? where? etc) or just on the info page.

I also added a "LIBREDDIT_DEFAULT_HIDE_AWARDS" option for a default - in case a server operator wants to hide it by default (goes with #678).

build.rs Outdated Show resolved Hide resolved
@sigaloid sigaloid force-pushed the instance-info-624 branch 2 times, most recently from da71ab4 to cb310e8 Compare January 11, 2023 18:45
@sigaloid
Copy link
Member Author

sigaloid commented Jan 11, 2023

Update: Added an HTML based endpoint

image

As well as a banner config option, for us to implement visible banners later.

I also added a fallback for the build.rs, for if the user doesn't have git. Sorry this PR is growing - each of the additional changes I've made were required for the core PR.

templates/base.html Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
src/instance_info.rs Outdated Show resolved Hide resolved
src/instance_info.rs Outdated Show resolved Hide resolved
src/instance_info.rs Outdated Show resolved Hide resolved
src/instance_info.rs Outdated Show resolved Hide resolved
src/instance_info.rs Outdated Show resolved Hide resolved
templates/base.html Outdated Show resolved Hide resolved
templates/base.html Outdated Show resolved Hide resolved
@Daniel-Valentine Daniel-Valentine dismissed their stale review January 12, 2023 09:17

Accidental approval -- comments still stand.

Copy link
Member

@Daniel-Valentine Daniel-Valentine left a comment

Choose a reason for hiding this comment

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

(leaving a comment to convince GitHub that I'm requesting changes, which I am)

@spikecodes
Copy link
Collaborator

I've removed the hard-coded colors from the .info-button in favor of the style-agnostic --text color variable so it'll show in dark themes as well. I've also aligned the footer to the far right so that the info link doesn't conflict (in terms of design hierarchy) with the Next/Previous buttons that are center-aligned. I also removed the border and box shadow because for non-interactive links (like Settings or Info), I think an underline makes more sense and helps differentiate from the buttons.

Otherwise looks great to me! I think Daniel had a couple more comments but I'm looking forward to this being merged.

src/instance_info.rs Outdated Show resolved Hide resolved
src/instance_info.rs Outdated Show resolved Hide resolved
templates/base.html Outdated Show resolved Hide resolved
src/instance_info.rs Outdated Show resolved Hide resolved
src/instance_info.rs Outdated Show resolved Hide resolved
src/instance_info.rs Show resolved Hide resolved
src/instance_info.rs Outdated Show resolved Hide resolved
@Daniel-Valentine Daniel-Valentine merged commit 8be5fde into master Jan 30, 2023
@Daniel-Valentine Daniel-Valentine deleted the instance-info-624 branch January 30, 2023 09:02
@sigaloid sigaloid mentioned this pull request May 31, 2023
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.

Detail instance info in a REST endpoint or existing page metadata
4 participants