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

Fix package_cache_async missing from ResolvedContext dict round-trip #1810

Conversation

nrusch
Copy link
Contributor

@nrusch nrusch commented Aug 7, 2024

This is a follow-up fix to #1679

The new package_cache_async attribute on the ResolvedContext class was not being round-tripped through the context serialization process. This leads to the following error after spawning a shell (when the rezolve context command runs):

17:44:39 ERROR    ResolvedContextError: Failed to load context from S:\Temp\rez_context_gsab2kwu\context.rxt: AttributeError: 'ResolvedContext' object has no attribute 'package_cache_async'

I will note that it seems a bit strange for ResolvedContext.from_dict to call context._update_package_cache() after creating a new instance, but I assume there are some historical reasons for this. However, given this pattern, if package_cache_async is set to False, from_dict could potentially block for a very long time.

CC @isohedronpipeline for visibility.

@nrusch nrusch requested a review from a team as a code owner August 7, 2024 01:11
@JeanChristopheMorinPerso JeanChristopheMorinPerso added this to the Next milestone Aug 7, 2024
Copy link

codecov bot commented Aug 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 58.52%. Comparing base (3d0f224) to head (fd541f5).
Report is 9 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1810   +/-   ##
=======================================
  Coverage   58.52%   58.52%           
=======================================
  Files         126      126           
  Lines       17206    17207    +1     
  Branches     3519     3519           
=======================================
+ Hits        10069    10070    +1     
  Misses       6468     6468           
  Partials      669      669           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@JeanChristopheMorinPerso JeanChristopheMorinPerso left a comment

Choose a reason for hiding this comment

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

Thanks!

@JeanChristopheMorinPerso JeanChristopheMorinPerso merged commit 8476696 into AcademySoftwareFoundation:main Sep 22, 2024
47 checks passed
Pixel-Minions added a commit to Pixel-Minions/rez that referenced this pull request Sep 26, 2024
…rip. Also bumped the serialization formation from 4.7 to 4.8 (AcademySoftwareFoundation#1810)

Signed-off-by: Nathan Rusch <[email protected]>
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.

2 participants