[WIP; broken] Use event_parser crate for natural language date parsing #12
Conversation
|
Error seems to be from this query: I don't know why I even wrote it to query directly like that. Not sure exactly why it's failing, though. Are you running MySQL 8 on your local? |
| @@ -25,6 +25,11 @@ rand = "0.8" | |||
| levenshtein = "1.0" | |||
| sqlx = { version = "0.6", features = ["runtime-tokio-rustls", "macros", "mysql", "bigdecimal", "chrono"]} | |||
| base64 = "0.13" | |||
| event_parser = { git = "https://github.com/allquixotic/event-parser", branch = "icalendar-bump" } | |||
There was a problem hiding this comment.
I'll probably create a fork of this branch under this GitHub org, just to keep everything in one place
There was a problem hiding this comment.
Sounds good. The icalendar bump was the minimum work needed just to get this to compile, run, and return a reasonable result, but if you intend to (like you hinted in Discord) maintain event_parser longer term and add features, this is a great idea.
There was a problem hiding this comment.
Perhaps not fully maintain, but it certainly opens up some more possibilities if tweaks are required :)
| pub async fn natural_parser(time: &str, _timezone: &str) -> Option<i64> { | ||
| println!("natural_parser of {}", time); | ||
| let evt = to_event(time); | ||
| println!("to_event: {}", evt.to_string()); |
There was a problem hiding this comment.
Worth either removing these or switching to "info!"/"debug!" before merging (I know this is still WIP, just commenting so its not forgotten :))
There was a problem hiding this comment.
For sure. I'll update them to debug!.
I'm running MariaDB 10.6.x on Ubuntu 22.04. Sorry; the instructions didn't specify MySQL specifically (vs. MariaDB), and there are quite some differences between the two products now. I had to adjust that query for MariaDB support, but I'm not even sure if my fix was correct, or if I should abandon MariaDB support and just install Oracle MySQL. I used MariaDB because an up-to-date version is available in the Ubuntu package manager without a third-party package; and because of the preferential licensing and support situation for MariaDB vs. MySQL (in my opinion). |
|
Ah okay, really it'd be good to avoid totally non-standard SQL stuff that will break Maria. I can have a look at modifying the query there maybe, or removing it entirely |
|
I also had issues with the migration queries that are used to create the DB. I might open a separate PR about that. One issue I recall was There was also an issue with a column being created with a default value of NULL for a timestamp column; I had to add |
Sorry, I didn't see this message. MySQL is a bit of a curse: realistically, I'd love to move to Postgres, but the amount of effort that requires is immense. I'd accept a PR to improve SQL compatibility, but it does need to remain compatible with MySQL 8 just because I have other software running on the server, and I don't want to run a third database server. I don't know the exact compatibility issues really. I can spin up a VM and go through and test some of this stuff |
Removing our dependency on forking and execing Python to call on the dateparser library, we can use the event_parser Rust crate which provides similar functionality (but not equivalent).
One limitation of event_parser that is immediately obvious vs. python-dateparser is the lack of non-English language support.
At this point, the
event_parsercode successfully produces an accurate UNIX timestamp and returns it from thetime_parser::natural_parser()for a few simple English-language test cases, likein 20 minutesor20 minutes ago.What doesn't work is something deep within reminderbot. You can find a panic stack trace here: https://gist.github.com/allquixotic/c908e043dff4ff9219463209756d0618
This stack trace is from
/remind time: in 20 minutes content: blah, and I verified throughprintln!()intime_parser::natural_parser()that a valid UNIX timestamp is being returned to the caller.This PR contains several crate version bumps, so that may have something to do with the crash.