modelmesh-serving
modelmesh-serving copied to clipboard
kserve
storage-secret-config should have the same parameters as Kserve
Is your feature request related to a problem? If so, please describe.
The json format of the storage-secret-config in ModelMesh expects the default bucket to be specified as follows : "default_bucket":"modelmesh-example-models"
However, this is not consistent with the Kserve implementation, which expects the default bucket to be specified as follows :
"bucket":"modelmesh-example-models".
This mismatch in the key is frustrating. Following a consistent format will allow users to have a better experience across both serving stacks (kserve and modelmesh) Describe your proposed solution
Match the storage-secret-config that Kserve has. i.e the key should be bucket rather than default_bucket
Hello, thanks for pointing this out! I think this is actually just a documentation problem
In the adapter code that processes the storage configuration, bucket is supported and takes priority over default_bucket (which is marked as deprecated), see these lines in the adapter repo. So I think it should "just work" if you use bucket, but let me know if you get an error!
If bucket does work, then we should see about replacing default_bucket -> bucket everywhere that it is used in this repo.
Sure, @AHB102. @tjohnson31415 gave some insight above, please ask if you need any clarification. More information about the storage-config secret can be found here.
@tjohnson31415 Can you please explain what exactly must be changed ? There are discrepancies with "bucket" and "default_bucket" in the json format of storage-secret-config in ModelMesh, right ? So we should just make one of them the standard ?
Hi @AHB102, as noted in the issue description, using "bucket" should be the standard to match with KServe, so that is already decided.
In this ModelMesh repo, the documentation and example files are using "default_bucket", see the GH search results to see where we reference it. As I noted in my previous comment, use of "bucket" should already be fully supported in the code. The changes for this issue would be to modify every place that "default_bucket" is referenced and change it to just "bucket".
As a validation of the change, you should also test that the quickstart install still works after modifying quickstart.yaml to use "bucket" instead of "default_bucket".
@tjohnson31415 Got it, Identify and replace every reference of "default_bucket" with "bucket" and then test quickstart installation to validate it. Can I work on it ?
Hi @AHB102, just wanted to confirm whether you still plan to work on this. I can assign it to you if so!
@rafvasq @AHB102
I can take this issue up if no one's currently working on it
