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.

Sign up for GitHub

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

Jump to bottom

fix bugs, rationalize code, support unix sockets #15

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix bugs, rationalize code, support unix sockets #15

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 1 addition & 1 deletion Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "rmmm"
version = "0.4.2"
version = "0.5.0"
description = "Rust MySQL Migration Manager"
repository = "https://github.com/EasyPost/rmmm"
authors = ["James Brown <[email protected]>"]
Expand Down
62 changes: 51 additions & 11 deletions src/go_database_dsn.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ static DSN_REGEX: Lazy<Regex> = Lazy::new(|| {
pub(crate) struct GoDatabaseDsn {
username: Option<String>,
password: Option<String>,
protocol: String,
Copy link

Choose a reason for hiding this comment

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

If I read this PR correctly, I think you're just supporting tcp and unix as protocols. It may be worth making this an enum. This would make the line if self.protocol == "unix" be more like match self.protocol { Protocol::Unix => {...}, Protocol::TCP => { ... } } which is cleaner. Looking at the list of mysql protocols, unix and tcp are the ones you'll care about so it's probably not worth it.

Copy link
Member Author

Choose a reason for hiding this comment

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

I considered that, but decided not to for that same reason.

address: Address,
database: String,
}
Expand All @@ -118,9 +119,15 @@ impl FromStr for GoDatabaseDsn {
let password = caps.name("password").map(|s| s.as_str().to_owned());
match caps.name("protocol").map(|s| s.as_str()) {
Some("tcp") => {}
Some("unix") => {}
Some(other) => anyhow::bail!("unhandled DSN protocol {}", other),
None => {}
}
let protocol = caps
.name("protocol")
.ok_or_else(|| anyhow::anyhow!("no protocol in DSN {}", s))?
.as_str()
.to_owned();
let address = caps
.name("address")
.ok_or_else(|| anyhow::anyhow!("no address in DSN {}", s))?
Expand All @@ -134,6 +141,7 @@ impl FromStr for GoDatabaseDsn {
Ok(GoDatabaseDsn {
username,
password,
protocol,
address,
database,
})
Expand All @@ -144,19 +152,28 @@ impl TryInto<mysql::Opts> for GoDatabaseDsn {
type Error = anyhow::Error;

fn try_into(self) -> Result<mysql::Opts, Self::Error> {
Ok(mysql::OptsBuilder::new()
.user(self.username)
.pass(self.password)
.db_name(Some(self.database))
.tcp_port(self.address.port)
.ip_or_hostname(Some(self.address.name.into_mysql_string()))
.into())
if self.protocol == "unix" {
Ok(mysql::OptsBuilder::new()
.user(self.username)
.pass(self.password)
.db_name(Some(self.database))
.socket(Some(self.address.name.into_mysql_string()))
.into())
} else {
Ok(mysql::OptsBuilder::new()
.user(self.username)
.pass(self.password)
.db_name(Some(self.database))
.tcp_port(self.address.port)
.ip_or_hostname(Some(self.address.name.into_mysql_string()))
.into())
}
}
}

#[cfg(test)]
mod tests {
use super::{Address, AddressName, GoDatabaseDsn};
use super::{Address, AddressName, GoDatabaseDsn, DEFAULT_PORT};
use std::net::{IpAddr, Ipv4Addr, Ipv6Addr};

use anyhow::Context;
Expand All @@ -183,7 +200,7 @@ mod tests {
"127.0.0.1".parse::<Address>().unwrap(),
Address {
name: AddressName::Address(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1))),
port: 3306,
port: DEFAULT_PORT,
}
);
assert_eq!(
Expand All @@ -197,7 +214,7 @@ mod tests {
"[::2]".parse::<Address>().unwrap(),
Address {
name: AddressName::Address(IpAddr::V6(Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 2))),
port: 3306,
port: DEFAULT_PORT,
}
);
assert_eq!(
Expand All @@ -207,6 +224,13 @@ mod tests {
port: 3307,
}
);
assert_eq!(
"/var/lib/mysql.sock".parse::<Address>().unwrap(),
Address {
name: AddressName::Name("/var/lib/mysql.sock".to_string()),
port: DEFAULT_PORT,
},
);
}

#[test]
Expand All @@ -221,6 +245,20 @@ mod tests {
port: 33606
}
);
let parsed: GoDatabaseDsn = "foo:bar@unix(/var/lib/mysql.sock)/foodb?ignored=true"
.parse()
.expect("should parse");
assert_eq!(
parsed.address,
Address {
name: AddressName::Name("/var/lib/mysql.sock".parse().unwrap()),
port: DEFAULT_PORT,
}
);
assert_eq!(
parsed.protocol,
"unix",
);
assert_eq!(parsed.username.as_deref(), Some("foo"));
assert_eq!(parsed.password.as_deref(), Some("bar"));
assert_eq!(parsed.database, "foodb".to_string());
Expand All @@ -229,7 +267,9 @@ mod tests {
"foo:bar@tcp([::1]:3300)/foo",
"foo@tcp([::1])/foo",
"tcp(127.0.0.1)/baz",
"usps:sekret@tcp(dblb.local.easypo.net:36060)/usps",
"user:sekret@tcp(hostname:36060)/dbname",
"user@unix(/var/lib/mysql/mysql.sock)/dbname?parseTime=true&loc=UTC&sql_mode='STRICT_ALL_TABLES,NO_ENGINE_SUBSTITUTION'",
"user:pass@unix(/var/lib/mysql/mysql.sock)/dbname?parseTime=true&sql_mode='STRICT_ALL_TABLES,NO_ENGINE_SUBSTITUTION'",
] {
s.parse::<GoDatabaseDsn>()
.context(format!("attempting to parse {}", s))
Expand Down
36 changes: 20 additions & 16 deletions src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ enum MigrationStatus {

#[derive(Tabled, Debug)]
struct MigrationStatusRow {
id: usize,
id: u32,
label: String,
status: MigrationStatus,
executed_at: String,
Expand All @@ -71,9 +71,9 @@ fn command_status(state: MigrationState, runner: MigrationRunner) -> anyhow::Res
let run_so_far = runner.list_run_migrations()?;
let all_ids = state
.all_ids()
.union(&run_so_far.iter().map(|m| m.id).collect::<BTreeSet<usize>>())
.union(&run_so_far.iter().map(|m| m.id).collect::<BTreeSet<u32>>())
.cloned()
.collect::<BTreeSet<usize>>();
.collect::<BTreeSet<u32>>();
let migrations_by_id = state.migrations_by_id();
let run_so_far_by_id = run_so_far
.into_iter()
Expand Down Expand Up @@ -111,15 +111,15 @@ fn command_status(state: MigrationState, runner: MigrationRunner) -> anyhow::Res

#[derive(Tabled, Debug)]
struct MigrationPlanRow {
id: usize,
prev_id: String,
id: u32,
sql_text: String,
}

fn command_upgrade(
fn command_apply_migrations(
matches: &clap::ArgMatches,
state: MigrationState,
runner: MigrationRunner,
is_upgrade: bool,
) -> anyhow::Result<()> {
debug!("Starting command_upgrade");
let target_revision = {
Expand All @@ -132,7 +132,7 @@ fn command_upgrade(
.context("revision must be an integer or 'latest'")?
}
};
let plan = runner.plan(&state, target_revision)?;
let plan = runner.plan(&state, target_revision, is_upgrade)?;
if plan.is_empty() {
info!("Nothing to do!");
return Ok(());
Expand All @@ -142,26 +142,25 @@ fn command_upgrade(
.iter()
.map(|ps| MigrationPlanRow {
id: ps.id,
prev_id: ps
.prev_id
.map(|u| u.to_string())
.unwrap_or_else(|| "(none)".to_string()),
sql_text: ps.sql.clone(),
})
.collect::<Vec<_>>();
let table = tabled::Table::new(&plan_data)
.with(tabled::Style::modern().horizontal_off())
.with(tabled::Modify::new(tabled::Column(2..=2)).with(tabled::Alignment::left()));
.with(tabled::Modify::new(tabled::Column(1..=1)).with(tabled::Alignment::left()))
;
println!("Migration plan:");
println!("{}", table);
if matches.is_present("execute") {
info!("executing plan with {} steps", plan.steps().len());
runner.execute(plan)?;
info!("done!");
println!("New version: {}", target_revision);
let schema = runner.dump_schema()?;
if !matches.is_present("no-dump") {
let schema = runner.dump_schema()?;
state.write_schema(&schema)?;
} else {
println!("not writing schema file");
}
} else {
error!("rerun with --execute to execute this plan");
Expand Down Expand Up @@ -296,7 +295,6 @@ fn cli() -> clap::Command<'static> {
Arg::new("no-dump")
.long("--no-write-schema")
.env("NO_WRITE_SCHEMA")
.action(clap::ArgAction::SetTrue)
.help("Do not write updated db/structure.sql when done"),
),
)
Expand All @@ -323,6 +321,12 @@ fn cli() -> clap::Command<'static> {
.short('x')
.long("execute")
.help("Actually upgrade (otherwise will just print what will be done"),
)
.arg(
Arg::new("no-dump")
.long("--no-write-schema")
.env("NO_WRITE_SCHEMA")
.help("Do not write updated db/structure.sql when done"),
),
)
.subcommand(
Expand Down Expand Up @@ -354,10 +358,10 @@ fn main() -> anyhow::Result<()> {
command_status(current_state, runner)?;
}
Some(("upgrade", smatches)) => {
command_upgrade(smatches, current_state, runner)?;
command_apply_migrations(smatches, current_state, runner, true)?;
}
Some(("downgrade", smatches)) => {
command_upgrade(smatches, current_state, runner)?;
command_apply_migrations(smatches, current_state, runner, false)?;
}
Some(("apply-snapshot", smatches)) => {
command_apply_snapshot(
Expand Down
Loading
Loading