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

Fix the logging path when running the GUI #3772

Merged
merged 1 commit into from
Aug 22, 2022

Conversation

frode-aarstad
Copy link
Contributor

@frode-aarstad frode-aarstad commented Aug 16, 2022

Issue
Resolves #3588

Approach
Add the logging path to the environment that is sent to the subprocess then use this in the custom file handler.

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.

@frode-aarstad frode-aarstad added the release-notes:improvement Automatically categorise as improvement in release notes label Aug 16, 2022
@frode-aarstad frode-aarstad changed the title Fix the loging path when running the GUI Fix the logging path when running the GUI Aug 16, 2022
@frode-aarstad frode-aarstad force-pushed the fix_log_path branch 2 times, most recently from a51f3d2 to 9d236d8 Compare August 16, 2022 12:46
@@ -55,7 +55,6 @@ def run_gui(args):
"workflow_jobs": str(res_config.model_config.getForwardModel().joblist())
},
)

Copy link
Contributor

Choose a reason for hiding this comment

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

not necessary change

filename, extension = os.path.splitext(filename)
filename = (
f"{filename}-"
f"{datetime.now().strftime(datetime.now().strftime('%Y-%m-%dT%H%M'))}"
f"{extension}"
)

if "use_log_dir_from_env" in kwargs:
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest removing this line and putting a default value of False in the pop method below.

@frode-aarstad frode-aarstad force-pushed the fix_log_path branch 2 times, most recently from 608aafd to 55feed5 Compare August 19, 2022 06:32
@oysteoh
Copy link
Contributor

oysteoh commented Aug 19, 2022

Jenkins: Test this please!

@frode-aarstad
Copy link
Contributor Author

Jenkins: Test this please!

@frode-aarstad frode-aarstad merged commit 4080cda into equinor:main Aug 22, 2022
@frode-aarstad frode-aarstad deleted the fix_log_path branch August 22, 2022 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes:improvement Automatically categorise as improvement in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make sure all logs are created with the same path
3 participants