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

Fixes exception thrown when pickling/unpickling ProductAttributesContainer #4271

Merged

Conversation

crgwbr
Copy link
Contributor

@crgwbr crgwbr commented Apr 4, 2024

After #4167 and prior to this patch, trying to use pickle with a Product or ProductAttributesContainer results in a recursion error.

@crgwbr crgwbr force-pushed the fix_pickling_product_attrs branch from 5cbbed4 to 5c70d02 Compare April 4, 2024 21:49
Copy link

codecov bot commented Apr 4, 2024

Codecov Report

Merging #4271 (204b9a7) into master (cb93f5e) will increase coverage by 0.44%.
Report is 11 commits behind head on master.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4271      +/-   ##
==========================================
+ Coverage   87.84%   88.28%   +0.44%     
==========================================
  Files         291      293       +2     
  Lines       16145    16046      -99     
==========================================
- Hits        14182    14166      -16     
+ Misses       1963     1880      -83     
Files Coverage Δ
src/oscar/apps/catalogue/product_attributes.py 98.13% <100.00%> (+0.08%) ⬆️

... and 10 files with indirect coverage changes

@crgwbr
Copy link
Contributor Author

crgwbr commented Apr 4, 2024

@specialunderwear The test failure here seems unrelated to my change, and a very similar issue seems to be occurring in master1. Any ideas on why?

Footnotes

  1. https://github.com/django-oscar/django-oscar/actions/runs/8510200510/job/23307293806

@specialunderwear
Copy link
Member

@specialunderwear The test failure here seems unrelated to my change, and a very similar issue seems to be occurring in master1. Any ideas on why?

Footnotes

  1. https://github.com/django-oscar/django-oscar/actions/runs/8510200510/job/23307293806

Yeas we have some ideas and we're working on it, but we can't reproduce this locally unfortunately, can you?

@crgwbr crgwbr force-pushed the fix_pickling_product_attrs branch from 5c70d02 to 204b9a7 Compare April 5, 2024 14:02
@crgwbr
Copy link
Contributor Author

crgwbr commented Apr 5, 2024

Yeas we have some ideas and we're working on it, but we can't reproduce this locally unfortunately, can you?

Unfortunately, no. The tests all pass for me locally.

@specialunderwear specialunderwear merged commit 2ea0c35 into django-oscar:master Apr 8, 2024
7 of 23 checks passed
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.

None yet

2 participants