-
Notifications
You must be signed in to change notification settings - Fork 1
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
fl server #62
fl server #62
Conversation
fa6120d
to
bd0b328
Compare
a48c2bd
to
17cd3c2
Compare
530f6db
to
230b0d3
Compare
6f217ec
to
df36948
Compare
3d4f18a
to
e588b80
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Beside the comments I left on the code, why do you always assume that the user database is a Vec. I mean what if we wanna change this to use a redis or other kind of db .
I think this part of code can be abstracted with a UserDB trait or something. First implementation can hold a vec of users from the configuraiton.
5a2deda
to
056aabf
Compare
…sit_dir_one_level functions
056aabf
to
2efda5e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good work @rawdaGastan :)
I left few comments on the code below, please check them out
…e json, use more functions in serve flists
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left few minor comments
fl-server/src/config.rs
Outdated
anyhow::bail!("host '{}' is invalid", c.host) | ||
} | ||
|
||
if c.port > 65535 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you make the port of u16 type instead of usize unwill not need this check since u16 MAX is 65535
fl-server/src/db.rs
Outdated
|
||
#[derive(Debug, ToSchema)] | ||
pub struct MapDB { | ||
users: Mutex<HashMap<String, User>>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do u need the Mutex ? I can see that the DB api only needs a shared ref
fl-server/src/db.rs
Outdated
.to_vec() | ||
.iter() | ||
.map(|u| (u.username.clone(), u.to_owned())) | ||
.collect(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
users.iter().cloned().collect()
Issues