feat: Force absolute paths in file based registries #4774
Closed
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
While I was working on #4772, I discovered an issue related to the Feast CLI and how
FileRegistryStoreinstances are populated when running a CLI command using the-fargument. Assume a really simple setup made with thefeast initcommand, i.e. the Feature Store is backed by file stores both in the registry and in the online store.When the registry is set to
filein thefeature_store.yaml,RegistryConfig'spathattribute is filled with the same exact value provided in the YAML, and at some point, theRegistryis instantiated. This is not the same for theFileRegistryStorethat is instantiated later, however - in that class, paths from theRegistryConfigare processed to turn them into absolute paths in the event they are relative:The problem here is that the tool used to build the CLI injects the current working directory in its context's
CHDIRkey, and thatCHDIRis passed down the successive calls as therepo_pathuntil theFileRegistryStoreis finally built, which results in a malformed absolute path. Consequently, things like the UI cannot load (the landing page just claims there was an issue while loading the project list due to a malformedfeature_store.yaml, but that's actually misleading).Besides these issues, there's another one related to the env var
FEATURE_STORE_YAML_BASE64. When the YAML is base-64 encoded and passed to the CLI with a file-based registry configured using a relative path, the final path built inFileRegistryStoretakes as prefix a subfolder of/tmp, hence assuming that the file registry is stored in that subfolder (which is impossible, as it is created on the fly just to store the decoded env var as an actual file somewhere). Once again, theFileRegistryStoreends up in a weird state.To mitigate these issues, I propose enforcing the usage of absolute paths while configuring the
feature_store.yaml, be it a file or an env var. The onus is on the user then, and I am aware this can be a breaking change for people currently using file-based registries on their Feature Store setups, but the odds of using registries like these in production environments are very low IMHO.Aside from that constraint, I also updated the logic behind the
feast initcommand to replace the default values of thefeature_store.yamltemplate with absolute paths built according to the dir from which the command was issued.Which issue(s) this PR fixes:
Misc