Skip to content

Fli/improve archive download#170

Merged
franknli merged 16 commits into
mainfrom
fli/improve_archive_download
Feb 4, 2025
Merged

Fli/improve archive download#170
franknli merged 16 commits into
mainfrom
fli/improve_archive_download

Conversation

@franknli
Copy link
Copy Markdown
Collaborator

No description provided.

@franknli franknli requested review from a team and PipKat as code owners December 18, 2024 15:17
@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Dec 18, 2024
self.run_script_string(script)
archive_created = self.run_script_string(script)
if not archive_created:
logging.error("failed to create the project archive. make sure that the solver PyAnsys sessions are closed.")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
logging.error("failed to create the project archive. make sure that the solver PyAnsys sessions are closed.")
logging.error("Failed to create the project archive. Make sure that the solver PyAnsys sessions are closed.")

Messages should use sentence case. They are to be brief, complete sentences.

Copy link
Copy Markdown
Member

@PipKat PipKat left a comment

Choose a reason for hiding this comment

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

One minor doc edit of a message string.

@franknli franknli requested a review from asawas January 8, 2025 15:39
archive_name : str
Name of the project archive to use, without the file extension.
include_solution_result_files : bool, default: True
Whether to include solution/result files in the archive.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The use of solution/result always leaves to the question of whether this is an "or" or "and" construction. It would be better to remove the / and write out the conjunction.

Comment on lines +416 to +417
"failed to create the project archive. "
"make sure that the solver PyAnsys sessions are closed."
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
"failed to create the project archive. "
"make sure that the solver PyAnsys sessions are closed."
"Failed to create the project archive. "
"Make sure that the solver PyAnsys sessions are closed."

You sentence construction--which means start with an initial cap. Otherwise, it is fine.


Examples
--------
Stop the PyMechanical session for the given system name.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
Stop the PyMechanical session for the given system name.
Stop the PyMechanical session for the given system.


Examples
--------
Stop the Fluent session for the given system name.
Copy link
Copy Markdown
Member

@PipKat PipKat Jan 8, 2025

Choose a reason for hiding this comment

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

Suggested change
Stop the Fluent session for the given system name.
Stop the Fluent session for the given system.


Examples
--------
Stop the Sherlock session for the given system name.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
Stop the Sherlock session for the given system name.
Stop the Sherlock session for the given system.

Copy link
Copy Markdown
Member

@PipKat PipKat left a comment

Choose a reason for hiding this comment

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

Approved doc strings with minor suggestions.

with open(file_path, mode="ab") as f:
f.write(response.file_content)

try_more = 3
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

How can we verify that the PermissionError is due to an intermittent error not due to some other reason (e.g., disk is full, wrong file/directory permissions, path issue, etc) ?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I guess we can't decide which is the case.

# intermittent "Permission Denied" error can happen
if (try_more <= 0):
throw
try_more -= 1
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

probably we should add a wait here to give enough time for intermittent error to clear.
e.g., time.sleep(0.01) # Wait for 10 milliseconds before retrying

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good idea. Done!

@franknli franknli merged commit 98db9dd into main Feb 4, 2025
@franknli franknli deleted the fli/improve_archive_download branch February 4, 2025 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants