-
Notifications
You must be signed in to change notification settings - Fork 33
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 prov files not printing with links in prov
index.html
#937
Conversation
- Update `save_provenance()` to generate `index.html` last - Update order of `save_pronenance()` child functions for readability
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @chengzhuzhang, this PR is ready for review.
The description contains all of the useful information about the changes. Thanks!
e3sm_diags/e3sm_diags_driver.py
Outdated
prov_dir = os.path.join(results_dir, "prov") | ||
|
||
paths: ProvPaths = { | ||
"log_path": os.path.join(prov_dir, LOG_FILENAME), | ||
"parameter_files_path": None, | ||
"python_script_path": None, | ||
"env_yml_path": None, | ||
"index_html_path": None, | ||
} | ||
|
||
paths["parameter_files_path"] = _save_parameter_files(prov_dir, parser) | ||
paths["python_script_path"] = _save_python_script(prov_dir, parser) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prov paths are now captured here to consolidate them under _log_diagnostic_run_info()
.
f.write(output.decode("utf-8")) | ||
logger.info("Saved environment yml file to: {}".format(fnm)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove unnecessary logger call.
f.write(cmd_used) | ||
logger.info("Saved command used to: {}".format(fnm)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove unnecessary logger call.
filepath = args.parameters | ||
elif hasattr(args, "other_parameters") and args.other_parameters: | ||
filepath = args.other_parameters[0] | ||
|
||
if not os.path.isfile(filepath): | ||
logger.warning("File does not exist: {}".format(filepath)) | ||
else: | ||
with open(filepath, "r") as f: | ||
contents = "".join(f.readlines()) | ||
|
||
# Remove any path, just keep the filename. | ||
new_filepath = filepath.split("/")[-1] | ||
new_filepath = os.path.join(results_dir, new_filepath) | ||
|
||
with open(new_filepath, "w") as f: | ||
f.write(contents) | ||
|
||
return new_filepath |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactor this code to remove repeating code.
subprocess.check_output( | ||
["git", "rev-parse", "--abbrev-ref", "HEAD"], | ||
cwd=os.path.dirname(__file__), | ||
stderr=subprocess.DEVNULL, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suppress git
command error if a versioned release is installed a conda environment instead.
subprocess.check_output( | ||
["git", "rev-parse", "HEAD"], | ||
cwd=os.path.dirname(__file__), | ||
stderr=subprocess.DEVNULL, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suppress git
command error if a versioned release is installed a conda environment instead.
def _log_diagnostic_run_info(self, log_path: str): | ||
"""Logs information about the diagnostic run. | ||
|
||
This method is useful for tracking the provenance of the diagnostic run | ||
and understanding the context of the diagnostic results. | ||
|
||
It logs the following information: | ||
- Timestamp of the run | ||
- Version information (Git branch and commit hash or module version) | ||
|
||
Parameters | ||
---------- | ||
log_path : str | ||
The path to the log file, which is stored in the `results_dir` | ||
sub-directory called "prov". | ||
|
||
Notes | ||
----- | ||
The version information is retrieved from the current Git branch and | ||
commit hash. If the Git information is not available, it falls back | ||
to the version defined in the `e3sm_diags` module. | ||
""" | ||
timestamp = datetime.now().strftime("%Y-%m-%d %H:%M:%S") | ||
|
||
try: | ||
branch_name = ( | ||
subprocess.check_output( | ||
["git", "rev-parse", "--abbrev-ref", "HEAD"], | ||
cwd=os.path.dirname(__file__), | ||
) | ||
.strip() | ||
.decode("utf-8") | ||
) | ||
commit_hash = ( | ||
subprocess.check_output( | ||
["git", "rev-parse", "HEAD"], cwd=os.path.dirname(__file__) | ||
) | ||
.strip() | ||
.decode("utf-8") | ||
) | ||
version_info = f"branch {branch_name} with commit {commit_hash}" | ||
except subprocess.CalledProcessError: | ||
version_info = f"version {e3sm_diags.__version__}" | ||
|
||
logger.info( | ||
f"\n{'=' * 80}\n" | ||
f"E3SM Diagnostics Run\n" | ||
f"{'-' * 20}\n" | ||
f"Timestamp: {timestamp}\n" | ||
f"Version Info: {version_info}\n" | ||
f"Log Filepath: {log_path}\n" | ||
f"{'=' * 80}\n" | ||
) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to e3sm_diags/e3sm_diags_driver.py
prov
index.htmlprov
index.html
Note, build is failing because of Codacy. I disabled the web hook for now. We can consider using it later. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for a quick, fix! I tested with PR with one run and all the provenance files are captured.
Maybe a small improvement to make: for the |
@chengzhuzhang I think you accidentally closed this.
What should be improved here? I think I launched the script using VS Code interactive console so it looks different compared to the normal |
in my test, I saw this: https://web.lcrc.anl.gov/public/e3sm/zhang40/cdat-migration-fy24/25-02-13-branch-viewer-fix937/prov/cmd_used.txt. |
The We can think of printing this nicely in another issue if it is easy and desired. Merging this PR now. |
Description
prov
pathse3sm_diags/e3sm_diags_driver.py
,e3sm_diags/run.py
git
command when the package is a version builde3sm_diags/run.py
Checklist
If applicable: