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: Crud operations completed #6

Closed
wants to merge 10 commits into from
Closed

Feat: Crud operations completed #6

wants to merge 10 commits into from

Conversation

aryansri-19
Copy link

No description provided.

GetPsyched and others added 6 commits June 25, 2024 20:31
@GetPsyched GetPsyched mentioned this pull request Jul 2, 2024
Copy link
Member

@GetPsyched GetPsyched left a comment

Choose a reason for hiding this comment

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

All of the interaction with the client should be done as methods implemented on Task. So, all the respective code should be moved from src/bin/todo/*.rs to src/lib.rs.

Also, please run just lint and just clippy before committing to avoid more back and forth.

src/db/mod.rs Outdated

pub async fn get_client() -> Result<&'static Client, Error> {
CLIENT.get_or_try_init(init_client).await
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
}
}

Copy link
Author

@aryansri-19 aryansri-19 Jul 2, 2024

Choose a reason for hiding this comment

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

whole crud operation in the lib.rs? or just the db operation?

src/db/mod.rs Outdated
use tokio_postgres::{Client, NoTls, Error};
use dotenv::dotenv;
use std::env;
use tokio::sync::OnceCell as TokOnceCell;
Copy link
Member

Choose a reason for hiding this comment

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

Why are we renaming this?

Copy link
Author

Choose a reason for hiding this comment

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

It was just in the docs, can use just as it is tho.

Comment on lines +46 to +78
impl ToSql for Priority {
fn to_sql(
&self,
_ty: &Type,
out: &mut BytesMut,
) -> Result<IsNull, Box<dyn StdError + Send + Sync>> {
match self {
Priority::Low => out.extend_from_slice(b"low"),
Priority::Medium => out.extend_from_slice(b"medium"),
Priority::High => out.extend_from_slice(b"high"),
}
Ok(IsNull::No)
}

fn accepts(ty: &Type) -> bool {
ty.name() == "priority"
}

tokio_postgres::types::to_sql_checked!();
}

impl FromStr for Priority {
type Err = String;

fn from_str(s: &str) -> Result<Self, Self::Err> {
match s.to_lowercase().as_str() {
"low" => Ok(Priority::Low),
"medium" => Ok(Priority::Medium),
"high" => Ok(Priority::High),
_ => Err(format!("Invalid priority: {}", s)),
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Can this be avoided? Given that now there is a Display trait for Priority now

Copy link
Author

Choose a reason for hiding this comment

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

It was an error because of the custom type "enum" in database, which was not recognized by tokio as it needed to convert string to enum type and enum type to string, read from the cli.

Copy link
Author

@aryansri-19 aryansri-19 Jul 2, 2024

Choose a reason for hiding this comment

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

if it's resolved this way, great. but i had to implement the same for Difficulty as well

Copy link
Member

Choose a reason for hiding this comment

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

Is the Display trait not enough?

Copy link
Author

Choose a reason for hiding this comment

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

I dont think so, because after the Display trait was implemented, i still got the error in the tokio while reading the enum value of difficulty

new_deadline: NaiveDateTime,
}

pub async fn command(_args: &Args) {
Copy link
Member

Choose a reason for hiding this comment

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

The underscore in _args specifies that it's an unused variable. If we're using it, remove the underscore.

Copy link
Author

Choose a reason for hiding this comment

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

Oh, okay

#[clap(long)]
new_priority: Priority,
#[clap(long, value_parser=mindmap::parse_datetime)]
new_deadline: NaiveDateTime,
Copy link
Member

Choose a reason for hiding this comment

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

Let's keep the deadline as NaiveDate; in hindsight, I don't see the time being useful for this since that's too much micro-management

Copy link
Author

Choose a reason for hiding this comment

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

Sure

@GetPsyched
Copy link
Member

Moved to #8

@GetPsyched GetPsyched closed this Jul 5, 2024
@GetPsyched GetPsyched deleted the feat/db-init branch July 5, 2024 08:25
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.

2 participants