implements overlay-configs options for tembo apply #466
Conversation
|
Closes Linear-2162 |
joshuajerin
requested review from
shahadarsh,
vrmiguel,
DarrenBaldwin07 and
sjmiller609
as code owners
…nto overlay-configs
…nto overlay-configs
There was a problem hiding this comment.
Looking great, just a few things. I think it looks like maybe the overlay will reset the instance settings to default if not specified in the overlay config, so I want to see a test for that. Also there are some files that don't need to be committed that could be cleaned up. To get the lint rules to pass in CI, run cargo clippy --fix --all and run cargo fmt.
…nto overlay-configs
tembo-cli/src/cmd/apply.rs
Outdated
| info!("Running validation!"); | ||
| super::validate::execute(verbose)?; | ||
| info!("Validation completed!"); | ||
|
|
||
| let env = get_current_context()?; | ||
|
|
||
| let instance_settings = get_instance_settings(_merge_path.clone())?; | ||
| println!("Instance settings: {:?}", instance_settings); |
There was a problem hiding this comment.
You can add a println in the get_instance_settings function towards the end saying overlay file merged successfully. I don't think you need to print the values but I'm fine keeping it if you think it will be helpful and just move it to get_instance_settings function.
There was a problem hiding this comment.
get_instance_settings is called in delete.rs as well. I felt that the user wouldn't want the instance settings to show up if he calls delete.rs and also that the user would like to see the merged instances together.
There was a problem hiding this comment.
Thanks for creating the .gitignore.
…nto overlay-configs
shahadarsh
changed the title
There was a problem hiding this comment.
Great work @joshuajerin. Not an easy task for your 1st one. I did some basic testing and local & cloud work as expected. There are a few other improvements we can do especially in how the merge logic is written but I'm fine merging this and doing a follow up PR with improvements.
https://linear.app/tembo/issue/TEM-2162/tembo-apply-overlay-configs