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

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

Merged
merged 1 commit into from
Sep 28, 2024

Conversation

cpitclaudel
Copy link
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
Copy link

snh commented Sep 21, 2024

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

@justinmayer
Copy link
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
Copy link
Contributor Author

cpitclaudel commented Sep 21, 2024

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
Copy link
Member

@justinmayer justinmayer left a comment

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
16 checks passed
@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