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

Adding instance software and version. Fixes #2222 #2733

Merged
merged 5 commits into from Feb 18, 2023

Conversation

dessalines
Copy link
Member

@Nutomic Is there any other info you think might be useful to store in the instance table?

@@ -71,7 +71,7 @@ tracing-error = "0.2.0"
tracing-log = "0.1.3"
tracing-subscriber = { version = "0.3.15", features = ["env-filter"] }
url = { version = "2.3.1", features = ["serde"] }
reqwest = { version = "0.11.12", features = ["json"] }
reqwest = { version = "0.11.12", features = ["json", "blocking"] }
Copy link
Member Author

Choose a reason for hiding this comment

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

This is unfortunately necessary, because our scheduler cannot work with async functions.

pub blocked: Option<Vec<String>>,
pub linked: Vec<Instance>,
pub allowed: Option<Vec<Instance>>,
pub blocked: Option<Vec<Instance>>,
Copy link
Member Author

Choose a reason for hiding this comment

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

Might as well return all the type information, and let front-ends decide what they want to use.

-- Add Software and Version columns from nodeinfo to the instance table

alter table instance add column software varchar(255);
alter table instance add column version varchar(255);
Copy link
Member Author

Choose a reason for hiding this comment

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

The front ends can just display Unknown or something if the field is missing.

src/lib.rs Outdated
@@ -111,6 +106,11 @@ pub async fn start_lemmy_server() -> Result<(), LemmyError> {
.timeout(REQWEST_TIMEOUT)
.build()?;

let scheduler_client = reqwest::blocking::Client::builder()
Copy link
Member Author

Choose a reason for hiding this comment

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

A blocking version of reqwest

@@ -33,6 +41,11 @@ pub fn setup(db_url: String) -> Result<(), LemmyError> {
clear_old_activities(&mut conn);
});

update_instance_software(&mut conn_2, &client);
scheduler.every(1.days()).run(move || {
update_instance_software(&mut conn_2, &client);
Copy link
Member Author

Choose a reason for hiding this comment

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

Runs on startup, and every day. Seems like a daily run is best. The software is never gonna change, but the software version might.

async fn test_nodeinfo() {
let client = Client::builder().build().unwrap();
let lemmy_ml_nodeinfo = client
.get("https://lemmy.ml/nodeinfo/2.0.json")
Copy link
Member Author

Choose a reason for hiding this comment

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

This was useful to test a few, but I'm not sure if this should be included.

@@ -1,6 +1,8 @@
#!/bin/bash
set -e

cd ../
Copy link
Member

Choose a reason for hiding this comment

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

I usually call ./scripts/fix-clippy.sh so that would break. You can use this then it should work from any path.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

.iter()
.map(|i| i.domain.clone())
.collect::<Vec<String>>()
.contains(&domain)
Copy link
Member

Choose a reason for hiding this comment

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

Should use .find()

Copy link
Member Author

Choose a reason for hiding this comment

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

clippy changed this to .any

.iter()
.map(|i| i.domain.clone())
.collect::<Vec<String>>()
.contains(&domain)
Copy link
Member

Choose a reason for hiding this comment

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

.find()

Copy link
Member Author

Choose a reason for hiding this comment

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

clippy changed this to .any

@@ -120,3 +133,80 @@ fn drop_ccnew_indexes(conn: &mut PgConnection) {
.execute(conn)
.expect("drop ccnew indexes");
}

// You can't use the structs from node_info.rs,
// because many fields are missing from other platforms.
Copy link
Member

Choose a reason for hiding this comment

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

I dont like this, better to mark fields as optional and use #[serde(default)] attribute in the original structs. Then you can also store user numbers etc if they are present.

Copy link
Member Author

Choose a reason for hiding this comment

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

K done.

.user_agent(user_agent)
.timeout(REQWEST_TIMEOUT)
.build()
.expect("couldnt build reqwest client");
Copy link
Member

Choose a reason for hiding this comment

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

Dont build client in the loop.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@Nutomic
Copy link
Member

Nutomic commented Feb 15, 2023

Looks good. But this is a breaking change so we shouldnt release it before 0.18.0

@dessalines
Copy link
Member Author

dessalines commented Feb 18, 2023

Looks good. But this is a breaking change so we shouldnt release it before 0.18.0

That's no problem, I got some other breaking changes coming in too. If we want to do any other 0.17 releases we can just cherry pick them to the 0.17-release branch.

@Nutomic Nutomic force-pushed the instance_software_and_version branch from dab5bb8 to b0dfa9f Compare February 18, 2023 13:22
@Nutomic Nutomic enabled auto-merge (squash) February 18, 2023 13:22
@dessalines dessalines merged commit 3735c6f into main Feb 18, 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.

None yet

2 participants