Add support for local binary distribution of resolver#120
Add support for local binary distribution of resolver#120tadeas-hejnic wants to merge 30 commits into
Conversation
…lver paths per app and platform, bypassing lakeFS downloads - Add get_local_resolver_path utility to match resolver by app name, alias, and platform - Update pre_resolver_init hook to prefer local resolver paths, support farm_publish launch type, and skip lakeFS on farm workers - Harden addon data JSON reading with error handling for missing/corrupt files - Add CollectResolverEnvVars publish plugin to forward resolver config vars (TF_DEBUG, logger settings) to farm job environments
| "AYONLOGGERLOGLVL": resolver_cfg.get("ayon_log_lvl", ""), | ||
| "AYONLOGGERSFILELOGGING": resolver_cfg.get( | ||
| "ayon_file_logger_enabled", "" | ||
| ), | ||
| "AYONLOGGERSFILEPOS": resolver_cfg.get( | ||
| "file_logger_file_path", "" | ||
| ), | ||
| "AYON_LOGGIN_LOGGIN_KEYS": resolver_cfg.get( | ||
| "ayon_logger_logging_keys", "" | ||
| ), |
There was a problem hiding this comment.
Flagging this because it still uses the old variable names. It should be updated once this PR is merged. If this PR gets merged first, then the fix should be made in that PR instead.
Nice catch! I added to the next commit. You can check it now |
BigRoy
left a comment
There was a problem hiding this comment.
I think the distinction between whether on farm or not is just making things more confusing (and dangerous) because this may mean the farm machines would end up picking a different resolver than the one being downloaded locally on the user's machine. As such, I think instead the settings should behave in that you can choose to either enable 'distributing' from LakeFS or locally or not at all (regardless of whether it's farm or not).
If farm is having issues with LakeFS, then really we should solve that. And not somehow have only farm machines behave differently in what resolver they may end up using.
We'll need to ensure production has a ground truth on behavior, ensuring that all launches have a reproducible setup on how they launch, regardless of whether it's farm or local etc.
I like the fact that we make it easier with this PR to offer a resolver from a local path.
I personally feel more for looking into a way to "share" the resolver by e.g. having even LakeFS distributed resolver to e.g. be downloaded and searched for on a network share. So you can customize the path on where the resolver should be, and if it's found - then even the farm machines would allow to pick that one up. However, it'd require concurrency checks on the downloads if timestamps mismatch, etc. and unique folders per resolver within that folder, etc. More complicated on how it deploys - but would make it so that not every machine needs to download from LakeFS individually if another had already downloaded it to the shared location in the studio.
There was a problem hiding this comment.
I think we should NOT be passing on the env var of TF_DEBUG and logger level to the job - especially because e.g. the file location may not at all be the same path that is available on a farm machine. The farm job should really be picking up the env vars in the same manner they would when running locally which I suspect is from settings. As such, I have my doubts about adding this collector.
|
@BigRoy the idea from our side is that if you're using the local resolvers, you're not using lakefs. The distinction between it being on the farm and not is complicating it unnecessarily I agree, our original thinking shifted a little as we worked on this . The way we're currently using it is essentially the way you're saying. We are using a local resolver everywhere, we aren't using a separate LakeFS binary for workstations, we are pointing every single client to the local resolver. So in my mind if local distribution is on, "Prefer Local" should always be on too. @tadeas-hejnic if you'd like help let me know |
|
That makes sense. To avoid confusion or any behaviour that can't be reproduced, I can remove the "Prefer toggle" and make it always enabled, so it will always override the LakeFS distribution. I'm just not sure how it should behave when the variant is defined in LakeFS, but not in the local distribution. Should it take the LakeFS one, fail or something else? |
We should make it so that either LakeFS distribution is enabled OR local distribution. Probably best using an enum attribute with
|
|
So I changed it based on @BigRoy comment. I have added also support for defining roots for the resolver paths. LakeFS distribution:
.
.
|
BigRoy
left a comment
There was a problem hiding this comment.
These settings changes will need server side settings conversion logic, so that existing overrides are preserved.
There was a problem hiding this comment.
I'm not convinced about this logic. I'd rather have it purely based on settings (which is the entry point to this anyway).
I think we're just mixing the set up USD resolver pre hook with the setting these env vars hook. I'd actually separate those to two hooks, and make the set debug level etc. hook just always run instead of only if local render.
Co-authored-by: Roy Nieterau <roy_nieterau@hotmail.com>
Co-authored-by: Roy Nieterau <roy_nieterau@hotmail.com>
The settings conversion is now implemented in the last commit |




Changelog Description
This pull request introduces support for using locally installed USD resolver binaries as an alternative to downloading them from LakeFS, with new configuration options in both the server and client. The resolver selection logic has been refactored to prioritize local resolvers when preferred or on farm jobs, and to fall back to LakeFS or local as needed. Additionally, a new plugin collects relevant environment variables for farm jobs, and the version has been updated.
Additional review information
The base of the code changes comes from this PR, but the current implementation improves the feature and make the local distribution support more general.
Testing notes:
lakefsdistributionenable/disablecombinations