Skip to content

Preparatory refactoring of ExecutionContext #132

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
Closed
4 tasks done
alamb opened this issue Sep 12, 2024 · 0 comments · Fixed by #142
Closed
4 tasks done

Preparatory refactoring of ExecutionContext #132

alamb opened this issue Sep 12, 2024 · 0 comments · Fixed by #142

Comments

@alamb
Copy link
Contributor

alamb commented Sep 12, 2024 •
edited
Loading

This ticket tracks my high level plans to restructure the code a little

Once I am done with this stuff, I plan to start working on integrating other systems (like maybe hugging face) as a test
This was referenced Sep 12, 2024
Merged
Merged
matthewmturner pushed a commit that referenced this issue Sep 12, 2024
@alamb
Extract ExecutionContext to its own module and cli code to CliApp (…
652ac9a 
…#131)

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:
1. Move ExecutionContext to its own module
2. Make fields not `pub`
3. Encapsulate the CLI code into its own module
@alamb alamb mentioned this issue Sep 12, 2024
Closed
matthewmturner pushed a commit that referenced this issue Sep 12, 2024
@alamb
Separate display logic from execution logic in ExecutionContext (#134)
493f577 
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.
@alamb alamb mentioned this issue Sep 12, 2024
Merged
matthewmturner pushed a commit that referenced this issue Sep 13, 2024
@alamb
Make code organization match the functional structure (#139)
b5183ad 
part of #132

We are moving `dft` so there is an engine and 2 uis (the terminal tui
and the cli)

Let's make the code structure match this functional structure:
1. Rename `ui` to `tui` to make it clear it is the terminal ui
2. Move the CLI related code into its own module (cli)
3. Move the command line parsing structures into `DftArgs` in in main.rs


I don't fully understand the split between what logic belongs in `app`
and what belongs in `tui` but I figure we can work on that split over
time
@alamb alamb changed the title Preparatory refactoring of Exectuion Preparatory refactoring of ExecutionContext Sep 14, 2024
@alamb alamb mentioned this issue Sep 14, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Implement Extension registration API alamb/datafusion-dft
1 participant
@alamb