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

Setup log file for job_dispatch logger #3999

Merged
merged 2 commits into from
Oct 18, 2022

Conversation

lars-petter-hauge
Copy link
Contributor

@lars-petter-hauge lars-petter-hauge commented Oct 5, 2022

By default python logging will go to stdout, which for calling job_dispatch will go into the void. We specify a logfile that currently we want placed into runpath (that is, one job_runner log file per realization).

Considering that we want to remove the abstraction of a runpath for the user we might want to reconsider where these logs eventually are placed.

Issue
Resolves #3998

Approach
Add logging configuration.

Pre review checklist

  • Added appropriate release note label
  • PR title captures the intent of the changes, and is fitting for release notes.
  • Commit history is consistent and clean, in line with the contribution guidelines.

Adding labels helps the maintainers when writing release notes. This is the list of release note labels.

@lars-petter-hauge lars-petter-hauge added the release-notes:misc Automatically categorise as miscellaneous change in release notes label Oct 5, 2022
@lars-petter-hauge lars-petter-hauge marked this pull request as ready for review October 5, 2022 11:18
@lars-petter-hauge lars-petter-hauge force-pushed the add_loghandler_jobdispatch branch 2 times, most recently from cc1270f to 33613f5 Compare October 6, 2022 06:10
@lars-petter-hauge
Copy link
Contributor Author

Getting error on type checking, but cannot recreate locally (updated mypy and friends).

Run mypy src/ert --config-file .mypy-strict.ini
[7](https://github.com/equinor/ert/actions/runs/3189642920/jobs/5203687522#step:7:8)
src/ert/logging/__init__.py:12: error: Function is missing a type annotation for one or more arguments

It's missing type annotations on *args and *kwargs, which I'm not certain what should be. Wondered if perhaps *kwargs could be dict[str, Any]. Funny thing is, my IDE already states this without type annotation, and if I add type annotation it says dict[str, dict[str, Any]], so apparently vscode already has some form of detection on positional and keywords arguments.

@xjules
Copy link
Contributor

xjules commented Oct 6, 2022

Getting error on type checking, but cannot recreate locally (updated mypy and friends).

Run mypy src/ert --config-file .mypy-strict.ini
[7](https://github.com/equinor/ert/actions/runs/3189642920/jobs/5203687522#step:7:8)
src/ert/logging/__init__.py:12: error: Function is missing a type annotation for one or more arguments

It's missing type annotations on *args and *kwargs, which I'm not certain what should be. Wondered if perhaps *kwargs could be dict[str, Any]. Funny thing is, my IDE already states this without type annotation, and if I add type annotation it says dict[str, dict[str, Any]], so apparently vscode already has some form of detection on positional and keywords arguments.

When I check our code we tend to use Any for both (which you did).

@lars-petter-hauge lars-petter-hauge force-pushed the add_loghandler_jobdispatch branch 2 times, most recently from ff61dea to c337742 Compare October 10, 2022 08:28
@codecov-commenter
Copy link

Codecov Report

Merging #3999 (55e9657) into main (23c8872) will decrease coverage by 0.88%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #3999      +/-   ##
==========================================
- Coverage   58.73%   57.85%   -0.89%     
==========================================
  Files         539      539              
  Lines       40101    40003      -98     
  Branches     3638     3600      -38     
==========================================
- Hits        23554    23143     -411     
- Misses      15533    15929     +396     
+ Partials     1014      931      -83     
Impacted Files Coverage Δ
src/ert/job_runner/cli.py 98.27% <100.00%> (+0.16%) ⬆️
src/ert/logging/__init__.py 100.00% <100.00%> (ø)
src/ert/shared/main.py 81.10% <100.00%> (-0.08%) ⬇️
src/clib/lib/analysis/update.cpp 2.98% <0.00%> (-40.74%) ⬇️
src/clib/lib/enkf/gen_kw_config.cpp 14.02% <0.00%> (-35.40%) ⬇️
src/clib/lib/enkf/enkf_node.cpp 1.36% <0.00%> (-31.86%) ⬇️
src/clib/lib/enkf/enkf_serialize.cpp 7.84% <0.00%> (-29.90%) ⬇️
src/clib/lib/enkf/gen_kw.cpp 2.63% <0.00%> (-24.76%) ⬇️
src/clib/lib/enkf/block_fs_driver.cpp 55.62% <0.00%> (-18.40%) ⬇️
src/clib/lib/enkf/summary_key_set.cpp 25.00% <0.00%> (-12.18%) ⬇️
... and 14 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@lars-petter-hauge lars-petter-hauge force-pushed the add_loghandler_jobdispatch branch 3 times, most recently from 767eb68 to ad9b934 Compare October 18, 2022 10:41
Make sure to clean up in case a test raises an exception. We could
either end up not cleaning up temporary directory, or even never joining
the thread and pytest will hang.
@lars-petter-hauge lars-petter-hauge force-pushed the add_loghandler_jobdispatch branch 2 times, most recently from ad2c058 to 85dfc86 Compare October 18, 2022 10:56
@lars-petter-hauge lars-petter-hauge changed the title Setup log configuration file for job_dispatch logger Setup log file for job_dispatch logger Oct 18, 2022
@lars-petter-hauge lars-petter-hauge self-assigned this Oct 18, 2022
Copy link
Contributor

@xjules xjules left a comment

Choose a reason for hiding this comment

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

🚀

@xjules xjules merged commit 29c0bcd into equinor:main Oct 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes:misc Automatically categorise as miscellaneous change in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Job_dispatch logger is not assigned a handler
3 participants