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

Developing a solution for u-cp647 #95

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Developing a solution for u-cp647 #95

wants to merge 13 commits into from

Conversation

ledm
Copy link
Collaborator

@ledm ledm commented Mar 23, 2023

Earlier this year, we changed how the area data is loaded by bgcval2, so that we use the local version of area in the ptrc, grid and diag nemo files, instead of relying on an old nemo meshmask file.

A problem occurred when we encountered u-cp647 which has "area_grid_T" and "area_grid_W" instead of "area". We now use "choose_best_var" to load the area data.

We also fix a problem where chmod fails when a folder was created by another user.

We also fix a problem when multiple users want to download the same jobID.

@ledm ledm marked this pull request as ready for review March 23, 2023 16:29
@ledm ledm requested a review from valeriupredoi March 23, 2023 16:29
bgcval2/download_from_mass.py Outdated Show resolved Hide resolved
os.chmod(outputFold, st.st_mode | stat.S_IWGRP)

try: os.chmod(outputFold, st.st_mode | stat.S_IWGRP)
except: pass
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This should be something, not except pass.

Copy link
Owner

Choose a reason for hiding this comment

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

fully agreed, maybe an OSError: could not change permissions on such and such?

@@ -483,7 +486,8 @@ def download_from_mass(
if auto_download:
shared_file_path = os.path.join(paths.shared_mass_scripts, os.path.basename(download_script_path))
print('writing file in shared path', shared_file_path)
shutil.copy(download_script_path, shared_file_path)
try: shutil.copy(download_script_path, shared_file_path)
except: pass
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This should be something, no except pass

Copy link
Owner

Choose a reason for hiding this comment

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

well, what'd you expect here to go wrong, maybe an IOError I think?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

EnvironmentError?

Copy link
Owner

Choose a reason for hiding this comment

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

that's the BaseClass of IOError and such, I've not used the base class ever - you need to put whatever that should raise - am not sure what, you'd know 😃

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not sure either - I don't make errors...

...

...

...

😆

Copy link
Owner

Choose a reason for hiding this comment

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

hahaha 🤣 Seriously now - whatever happens in there when there is an exception - make the thing fail and see what exception it throws

bgcval2/functions/tools.py Show resolved Hide resolved
bgcval2/functions/tools.py Outdated Show resolved Hide resolved
@ledm ledm requested a review from tillku March 23, 2023 16:32
Copy link
Owner

@valeriupredoi valeriupredoi left a comment

Choose a reason for hiding this comment

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

awesome, bud! Just a couple very small wiggles from me 🍺

bgcval2/download_from_mass.py Outdated Show resolved Hide resolved
bgcval2/download_from_mass.py Outdated Show resolved Hide resolved
@@ -483,7 +486,8 @@ def download_from_mass(
if auto_download:
shared_file_path = os.path.join(paths.shared_mass_scripts, os.path.basename(download_script_path))
print('writing file in shared path', shared_file_path)
shutil.copy(download_script_path, shared_file_path)
try: shutil.copy(download_script_path, shared_file_path)
except: pass
Copy link
Owner

Choose a reason for hiding this comment

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

well, what'd you expect here to go wrong, maybe an IOError I think?

bgcval2/functions/tools.py Outdated Show resolved Hide resolved
ledm and others added 4 commits March 23, 2023 17:20
Co-authored-by: Valeriu Predoi <valeriu.predoi@gmail.com>
Co-authored-by: Valeriu Predoi <valeriu.predoi@gmail.com>
Co-authored-by: Valeriu Predoi <valeriu.predoi@gmail.com>
@valeriupredoi valeriupredoi added the enhancement New feature or request label Mar 23, 2023
@valeriupredoi
Copy link
Owner

valeriupredoi commented Apr 18, 2023

@tillku mentioned to me yesterday that he's still facing troubles - Till, did you grab this branch and tested with it? If so, could you pls tell us what went wrong? 🍺

@valeriupredoi
Copy link
Owner

this is starting to smell like abandonware - shall we merge this @ledm ?

@valeriupredoi valeriupredoi changed the title Developping a solution for u cp647 Developing a solution for u-cp647 Aug 14, 2023
@valeriupredoi
Copy link
Owner

maybe make those excepts more except-y and merge this? @ledm 🍺

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