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.

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

Embed default config and attempt to fix tests #29

Merged
merged 2 commits into from
Feb 1, 2024

Conversation

AlexMikhalev
Copy link
Contributor

All tests are running using earthly +pipeline on earthly satellite.
Strangely, create article API tests are failing - the server can't add articles to the index.
I also changed default config - as a quick fix, we need to think how to make it in a sane way.

Signed-off-by: AlexMikhalev <[email protected]>
@AlexMikhalev AlexMikhalev requested a review from mre January 31, 2024 15:58
if filename.exists() {
log::warn!("File exists");
log::warn!("{:?}", filename);
} else {
log::warn!("File does not exist");
std::fs::copy("default/settings.toml", &filename)?;
// FIXME: the logic of settings that each app - desktop or server have their own default settings.toml
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mre I want your opinion on how to design it better - see FIXME: comment.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we could have an environment variable for the config path that could be overwritten.
Then we could use it here.

TERRAPHIM_CONFIG_PATH=...

It could be set to a sane default.
Just an idea.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What is the path variable for default cargo run and cargo test? We can put the default config into that path, and it will pick it up automatically for run and test, but then subfolders in the Axum server and desktop can have their own when run.

Copy link
Contributor

@mre mre left a comment

Choose a reason for hiding this comment

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

lgtm.
Good work with solid improvements. I vote for merging that and iterating on it from the main branch.

@AlexMikhalev AlexMikhalev merged commit 5140e31 into main Feb 1, 2024
0 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants