Skip to content

19 global logging #22

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

Open
wants to merge 19 commits into
base: develop
Choose a base branch
from
Open

19 global logging #22

wants to merge 19 commits into from

Conversation

kjczarne
Copy link
Contributor

@kjczarne kjczarne commented May 28, 2022

List all Issues this Pull Request is meant to solve:

Run through our contribution checklist:

  • Have you read kalash/doc/contributing.md?
  • Have you added tests for the implemented feature/fixed bug?
  • Have you regenerated the JSON Schema (nox -e json_schema, only if you did anything with the data model)?
  • Have you ran a full test suite (nox -e test)?
  • Have you checked if the test coverage is at least 85%?
  • Have you ran code style check (flake8)?
  • Have you documented the change?
  • Have you updated CHANGELOG.md?

Describe shortly the changes you have introduced:

  • Added a global logger that directly uses the Python logging interface, there is no need to wrap anything in my opinion. The global logger is a bit lower level since it will be predominantly used by the developers.
  • Made the configuration loadable from an internal-config.yaml. At the moment configuration preload is not happening at runtime because the scope of this change was just the logger but I needed the internal config for some smart handling of defaults that can be configured as per project. Users now have more control over how the per-test-case logs are grouped and how the file logs are named.

Add additional comments (if any):
The checklist is incomplete because I'll run through the rest for a number of tickets at once.

Krzysztof Czarnecki added 19 commits May 26, 2022 02:34
The interpolation system should be available to the users or at least test managers to configure the behavior of the tool.
I am already getting confused by all the multiple levels of configuration so it's better for general readability to name the original `config` file `model`, since it basically provides the data model
…nually atm

Whoever had the crazy idea to use YAML tags, should burn in hell. I wish there was something like `serde` for Python
…take

This will always load the YAML post-initialization and will override some of the values you've set in the constructor.
Hence this is wrong and you should only call `preload_configuration` on demand from `main`
The group needs to be fetched from `meta` and used in the formation of the log tree paths. This was not the case and we were ending up with the property name in the tree, e.g. `id` instead of the actual literal ID of the test cases.
Because `yaml` support in Python is frankly laughably bad and I only realized how bad after working with YAML in Rust
…lize as dict, luckily we can support both on `load`
@kjczarne kjczarne added the enhancement New feature or request label May 28, 2022
@kjczarne kjczarne added this to the v5.0.0 Release Milestone milestone May 28, 2022
@kjczarne kjczarne self-assigned this May 28, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #22 (19058aa) into develop (cdd6488) will decrease coverage by 1.50%.
The diff coverage is 74.16%.

@@             Coverage Diff             @@
##           develop      #22      +/-   ##
===========================================
- Coverage    76.83%   75.32%   -1.51%     
===========================================
  Files           14       15       +1     
  Lines          833      912      +79     
===========================================
+ Hits           640      687      +47     
- Misses         193      225      +32     
Impacted Files Coverage Δ
kalash/run.py 46.46% <7.69%> (-2.74%) ⬇️
kalash/filter.py 63.79% <66.66%> (ø)
kalash/interpolation.py 68.18% <68.18%> (ø)
kalash/model.py 88.52% <72.97%> (ø)
kalash/utils.py 93.33% <90.90%> (-6.67%) ⬇️
kalash/collectors.py 91.66% <100.00%> (ø)
kalash/kalash_test_loader.py 89.18% <100.00%> (ø)
kalash/log.py 91.20% <100.00%> (-2.70%) ⬇️
kalash/metaparser.py 92.10% <100.00%> (ø)
kalash/spec.py 97.43% <100.00%> (-2.57%) ⬇️
... and 3 more

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 cdd6488...19058aa. Read the comment docs.

@kjczarne
Copy link
Contributor Author

TODO:

  • CLI options for the new parameters added to the CliConfig class
  • Document the improvements to the logging system (global logger (dev), per-test-case loggers with more flexible configuration)
  • Complete the MR checklist

@kjczarne
Copy link
Contributor Author

This MR will partly solve #17 as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants