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

Add configurable history length #447

Merged
merged 2 commits into from
Jun 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions atuin-server/migrations/20220610074049_history-length.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
-- Add migration script here
alter table history alter column data type text;
Copy link
Member Author

Choose a reason for hiding this comment

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

Text has no max size, unlike varchar

25 changes: 21 additions & 4 deletions atuin-server/src/database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@ use async_trait::async_trait;
use chrono::{Datelike, TimeZone};
use chronoutil::RelativeDuration;
use sqlx::{postgres::PgPoolOptions, Result};
use tracing::{debug, instrument};
use tracing::{debug, instrument, warn};

use super::{
calendar::{TimePeriod, TimePeriodInfo},
models::{History, NewHistory, NewSession, NewUser, Session, User},
};
use crate::settings::Settings;
use crate::settings::HISTORY_PAGE_SIZE;

use atuin_common::utils::get_days_from_month;
Expand Down Expand Up @@ -61,18 +62,19 @@ pub trait Database {
#[derive(Clone)]
pub struct Postgres {
pool: sqlx::Pool<sqlx::postgres::Postgres>,
settings: Settings,
}

impl Postgres {
pub async fn new(uri: &str) -> Result<Self> {
pub async fn new(settings: Settings) -> Result<Self> {
let pool = PgPoolOptions::new()
.max_connections(100)
.connect(uri)
.connect(settings.db_uri.as_str())
.await?;

sqlx::migrate!("./migrations").run(&pool).await?;

Ok(Self { pool })
Ok(Self { pool, settings })
}
}

Expand Down Expand Up @@ -252,6 +254,21 @@ impl Database for Postgres {
let hostname: &str = &i.hostname;
let data: &str = &i.data;

if data.len() > self.settings.max_history_length
&& self.settings.max_history_length != 0
{
// Don't return an error here. We want to insert as much of the
// history list as we can, so log the error and continue going.

warn!(
"history too long, got length {}, max {}",
data.len(),
self.settings.max_history_length
);

continue;
}

sqlx::query(
"insert into history
(client_id, user_id, hostname, timestamp, data)
Expand Down
2 changes: 1 addition & 1 deletion atuin-server/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ pub mod settings;
pub async fn launch(settings: Settings, host: String, port: u16) -> Result<()> {
let host = host.parse::<IpAddr>()?;

let postgres = Postgres::new(settings.db_uri.as_str())
let postgres = Postgres::new(settings.clone())
Copy link
Member Author

Choose a reason for hiding this comment

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

Settings is also used below, and I'm lazy 🤐

.await
.wrap_err_with(|| format!("failed to connect to db: {}", settings.db_uri))?;

Expand Down
2 changes: 2 additions & 0 deletions atuin-server/src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ pub struct Settings {
pub port: u16,
pub db_uri: String,
pub open_registration: bool,
pub max_history_length: usize,
}

impl Settings {
Expand All @@ -33,6 +34,7 @@ impl Settings {
.set_default("host", "127.0.0.1")?
.set_default("port", 8888)?
.set_default("open_registration", false)?
.set_default("max_history_length", 8192)?
Copy link
Member Author

Choose a reason for hiding this comment

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

8192 is the old default set in the database schema, so behaviour does not change

.add_source(
Environment::with_prefix("atuin")
.prefix_separator("_")
Expand Down