Extract ExecutionContext to its own module and cli code to CliApp
#131
Conversation
There was a problem hiding this comment.
FYI @matthewmturner this is step one of a few PRs I have planned. I filed #132 with my rough plan of attack
| @@ -73,7 +73,7 @@ pub fn normal_mode_handler(app: &mut App, key: KeyEvent) { | |||
| let execution = Arc::clone(&app.execution); | |||
| let _event_tx = app.app_event_tx.clone(); | |||
| tokio::spawn(async move { | |||
| let client = &execution.flightsql_client; | |||
| let client = execution.flightsql_client(); | |||
There was a problem hiding this comment.
I just made the fields non pub and made accessors so the API surface area was clearer
There was a problem hiding this comment.
Yup makes sense, i had started to use that approach for more recent work but i know theres some remaining pub stuff
| "Cannot execute both files and commands at the same time" | ||
| )), | ||
| } | ||
| /// Encapsulates the command line interface |
There was a problem hiding this comment.
I refactored a bunch of free functions into a struct (which I plan to move to its own module soon)
| pub struct ExecutionContext { | ||
| pub session_ctx: SessionContext, |
There was a problem hiding this comment.
when I made these non pub it turned out much of it isn't used, so I removed the unused fields for now
|
|
||
| /// Structure for executing queries either locally or remotely (via FlightSQL) |
There was a problem hiding this comment.
This is the core reason for this PR -- to move this structure to its own module
It still depends on app::AppEvent as well as printing to stdout. I plan to move the display of results code into different structures in the next PR
alamb
changed the title
ExecutionContext to its own module
alamb
changed the title
ExecutionContext to its own moduleExecutionContext to its own module and cli code to CliApp
Second part of #132 Follow on to #131 The rationale here is to split out the logic that handles display (e.g. `println!` and `Query` from the actual execution, again so that we can update / test the underying `ExecutionContext` without having the UI affected This PR moves the "display" releated logic out of the ExecutionContext and into the app / CLI modules respectively.
Part of #132
This PR extracts the ExecutionContext into its own module, as a first step towards
adding additional features (e.g. other table formats)
Having it as its own module will allow us to test it more easily and keeps the boundaries of the software clearer.
Changes:
pub