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

Add package_cache_async flag. #1452

Conversation

ttrently
Copy link
Contributor

@ttrently ttrently commented Feb 22, 2023

Fixes #1379.

Adds a package_cachy_async flag which allows users to run caching synchronously (blocking) or asynchronously from the config.

This setting is set to True by default in the rezconfig so that behavior should act as normal. Setting this to false will block the resolve until all packages have been cached successfully.

@ttrently ttrently requested review from nerdvegas and a team as code owners February 22, 2023 17:07
@JeanChristopheMorinPerso
Copy link
Member

Thanks @ttrently ! Can you follow the instructions at https://github.com/AcademySoftwareFoundation/rez/pull/1452/checks?check_run_id=11526898738 to fix the DCO check please?

@alx00x
Copy link
Contributor

alx00x commented Feb 22, 2023

A welcome addition. I want to add it should be implemented as a flag on rez-env command and an argument for ResolvedContext in the same manner how package caching itself can be disabled or enabled explicitly (config being a fallback).

For example:

rez env my_package --no-async
context = ResolvedContext(["my_package"], async_package_caching=False)

Adds a `package_cachy_async` flag which allows users to run caching synchronously (blocking) or asynchronously from the config.

Signed-off-by: ttrently <41705925+ttrently@users.noreply.github.com>
Change _async to default True instead of False to mimic previous behavior.

Signed-off-by: ttrently <41705925+ttrently@users.noreply.github.com>
Renamed `add_variants_async` to `add_variants` as this method can now run with an `_async` flag.

Signed-off-by: ttrently <41705925+ttrently@users.noreply.github.com>
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 @ttrently ! I think it could be a good idea to have a CLI flag, but I'm not sure what it should be named and to which commands it should apply. Any opinions?

src/rez/package_cache.py Show resolved Hide resolved
@@ -269,6 +269,9 @@
# Enable package caching during a package build.
package_cache_during_build = False

# Enable package caching to run asynchronously during a resolve.

Choose a reason for hiding this comment

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

I find this hard to read. How about "set to false to always cache package payloads synchronously"?

@JeanChristopheMorinPerso
Copy link
Member

@JeanChristopheMorinPerso
Copy link
Member

JeanChristopheMorinPerso commented Mar 7, 2024

Closing in favor of #1679. @ttrently I'll make sure that you are marked as co-author.

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