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

added error message for parameter bug #41

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JackTemaki
Copy link
Contributor

I had a strange bug, where opening the pickle file would result in the whole config to be loaded again, resulting conflicting emtpy jobs. It took some time until I could find a wrong parameter as cause (it was a wrong class added to a dict which was part of a correct class passed as parameter, the inputs are unfortunately quite complicated sometimes).

@critias
Copy link
Contributor

critias commented Jun 29, 2020

I'm not sure if I understand the problem correctly. Something similar like this failed in your case:

class Foo(Job):
  ...

class Bar(Job):
  ...

test = Bar({'test': Foo()})
dump = pickle.dumps(test)
test_loaded = pickle.loads(dump)

Correct? The bugfix looks more like treating the symptoms and not the cause, but I need to understand the cause better.

P.S.: Sorry for the late response, I was on vacation.

@JackTemaki
Copy link
Contributor Author

JackTemaki commented Jun 29, 2020

So the issue was that during pickle.load, the whole Sisyphus config (starting from config/__init__py) was loaded again, which lead to errors. This must have been the case because some dependency was included in the pickle. Unfortunately I corrected everything, so I am not sure if I can produce this error again.

@curufinwe curufinwe requested review from curufinwe and michelwi May 17, 2023 07:16
@curufinwe
Copy link
Collaborator

Shall we merge @michelwi ? I see no problem adding the code, even if it's not relevant anymore.

@michelwi
Copy link
Contributor

Could we check for a more specific exception or reraise the original exception together with the explanation. Just so if any other error occured the user is not confused.

@JackTemaki
Copy link
Contributor Author

From my memory I think this was a 'job' as no attribute '_sis_alias_prefixes', and one can explicitly catch for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants