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

Cleanup config files #426

Merged
merged 9 commits into from Aug 5, 2020

Conversation

CasperWA
Copy link
Member

@CasperWA CasperWA commented Jul 22, 2020

Derived from #420

The only config file we had that works with the servers in this repository was the tests/test_config.json.
This is not very helpful to get this up and running locally to test it out, I think.

Instead I have moved that file to the root and renamed it to optimade_config.json, removing the other two superfluous config files in the root (example_config.json and default_config.json), which weren't used for anything other than examples.
I'm of the conviction that if the documentation doesn't fully explain the configuration options then a random JSON file is not going to help.

Furthermore, I updated the tests/test_config.json file to reflect the OptimadeTestClient used, i.e., a single change of base_url to the one used in OptimadeTestClient.

The new optimade_config.json is now set as the OPTIMADE_CONFIG_FILE environment variable in the CI pytests.
We should potentially check that after the tests have run, the environment variable hasn't changed, since that is another goal of some of the tests that checks config.py; to not tamper with the host environment.

Edit: Just found we actually have an issue for this, i.e., closes #310.


Missing (from review):

  • Configuration documentation

@codecov
Copy link

codecov bot commented Jul 22, 2020

Codecov Report

Merging #426 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #426   +/-   ##
=======================================
  Coverage   91.26%   91.26%           
=======================================
  Files          60       60           
  Lines        2815     2815           
=======================================
  Hits         2569     2569           
  Misses        246      246           
Flag Coverage Δ
#unittests 91.26% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
optimade/server/config.py 88.05% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dda77e9...55cd866. Read the comment docs.

@ml-evs
Copy link
Member

ml-evs commented Jul 22, 2020

I'm of the conviction that if the documentation doesn't fully explain the configuration options then a random JSON file is not going to help.

I certainly agree... fancy writing the config documentation as part of this? 😁

@ml-evs
Copy link
Member

ml-evs commented Jul 22, 2020

I'm of the conviction that if the documentation doesn't fully explain the configuration options then a random JSON file is not going to help.

I certainly agree... fancy writing the config documentation as part of this? grin

I'm not imagining something massive, just a page on "Configuration" that links to the auto-generated pydantic doc part and explains about the env var... the "use case" sections of the docs will have to expand on why certain options should be set and what they actually do.

@CasperWA CasperWA changed the title Cleanup config files [WIP] Cleanup config files Jul 22, 2020
@CasperWA CasperWA changed the title [WIP] Cleanup config files Cleanup config files Jul 22, 2020
@CasperWA CasperWA requested a review from shyamd August 3, 2020 10:07
New dependency: mkdocs-codeinclude-plugin v0.0.1
This allows the rendition/inclusion of code files, e.g., .py, .json,
.yml, etc.
This is used in the new configuration site to render
`optimade_config.json`.

Change docs site name and description from "OPTIMADE-Python-Tools" ->
"OPTIMADE Python tools" to align with the README title.

Use minify plugin to minify HTML files.

Set mkdocstring default_handler explicitly to `python`.

Remove configuration section from INSTALL.md.

Edit: A later rebase introduced usage of snippets instead.
There is no need for the dependency.
I have found out what PyMdown is :)
The same can be achieved using PyMdown's Tabbed and Snippets.

Furthermore, I have reverted some changes not relevant to this PR.
Change optimade_config.json maintainer email to dev@optimade.org
Update values and add in the new configuration options as well.

The version default_config.json will also be updated now when using the
invoke task `setver`.

default_config.json has also been added to the bash script
`update_docs.sh` run during publishing a new version.
@CasperWA CasperWA requested a review from ml-evs August 3, 2020 15:04
@CasperWA CasperWA merged commit e965bc8 into Materials-Consortia:master Aug 5, 2020
@CasperWA CasperWA deleted the cleanup_config_files branch August 5, 2020 10:01
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.

Configuration documentation
3 participants