Skip to content

Don't store a RichHandler object in the argparse namespace (#3394)#3398

Merged
justinmayer merged 1 commit into
getpelican:mainfrom
cpitclaudel:cpc/formatter-pickle
Sep 28, 2024
Merged

Don't store a RichHandler object in the argparse namespace (#3394)#3398
justinmayer merged 1 commit into
getpelican:mainfrom
cpitclaudel:cpc/formatter-pickle

Conversation

@cpitclaudel

Copy link
Copy Markdown
Contributor

Resolves: #3394

  • Ensured tests pass and (if applicable) updated functional test output
  • Conformed to code style guidelines by running appropriate linting tools
  • Added tests for changed code
  • Updated documentation for changed code

Unfortunately I can't test locally, but this patch should be harmless, at least.

@snh

snh commented Sep 21, 2024

Copy link
Copy Markdown

I too was getting the error described in #3394 and can confirm this patch fixes the issue for me.

@justinmayer

Copy link
Copy Markdown
Member

Thanks, Clément! Do you think it's worth adding a test that fails for MacOS images in CI when this change is not present?

@cpitclaudel

cpitclaudel commented Sep 21, 2024

Copy link
Copy Markdown
Contributor Author

A regression test would be nice, yes. I suppose the best way to go about it would be to start the file watcher and then stop it a few seconds later somehow, asserting that it didn't crash in the meantime? I don't know when I'll have the time to do that.

@justinmayer justinmayer changed the title Don't store a RichHandler object in the argparse namespace (#3394) Don't store a RichHandler object in the argparse namespace (#3394) Sep 28, 2024

@justinmayer justinmayer left a comment

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.

Many thanks for the fix, Clément. I'm going to merge this now so that we can release a fix for this and thereby eliminate the problem for end users as soon as possible. If you think of one or more unit tests that might prevent future regressions, submitting them in a follow-up pull request would be most appreciated. Thanks again!

@justinmayer justinmayer merged commit 0cb445c into getpelican:main Sep 28, 2024
@cpitclaudel cpitclaudel deleted the cpc/formatter-pickle branch September 29, 2024 09:47
offbyone added a commit to offbyone/ideas that referenced this pull request Oct 2, 2024
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.

Running pelican -r -l produces Pickle-related error

3 participants