search

microsoft / charts

A Helm Chart Repository for Microsoft Projects
MIT License
30 stars 27 forks source link

Rename speech-onprem chart and slim packaged file. #2

Closed anthonsu closed 5 years ago

anthonsu commented 5 years ago

As required, renamed the helm chart from speech-onprem to cognitive-services-speech-onpremise. Slim the helm chart packaged file by removing unnecessary files inside.

anthonsu commented 5 years ago

@BrendanMWalsh, @dbanda, @IEvangelist

IEvangelist commented 5 years ago

I agree with changing the name, but I'm now leaning away from the onprem verbiage. It's a helm chart that contains two speech services, why are we binding it to onprem? It can be installed in the cloud for example, and that would be a perfectly acceptable use case.

I suggest renaming it to: cognitive-services-speech

anthonsu commented 5 years ago

I agree with changing the name, but I'm now leaning away from the onprem verbiage. It's a helm chart that contains two speech services, why are we binding it to onprem? It can be installed in the cloud for example, and that would be a perfectly acceptable use case.

I suggest renaming it to: cognitive-services-speech

So this helm chart is specific for onpremise use case only right? And that is why it uses onpremise docker contain image inside. I created the dir to be cognitive-service-speech as a general name, and put the onprem tar under it. In future, if there is other speech helm chart, it can be definitely put under this same dir, probably named after cognitive-services-speech-XXX-version.tgz.

IEvangelist commented 5 years ago

Hey @anthonsu, I guess I didn't know if that was true or not. I thought that it was just the container images for each service, without any notion of onprem. If that is true, I'm fine with the name. It just kind of feels weird, how are the images different?

anthonsu commented 5 years ago

Hey @anthonsu, I guess I didn't know if that was true or not. I thought that it was just the container images for each service, without any notion of onprem. If that is true, I'm fine with the name. It just kind of feels weird, how are the images different?

Ya, the docker image inside this helm chart is the image for onprem use :) It basically wraps different layers for onprem use cases.

BrendanWalsh commented 5 years ago

In general our contract with customers is that the image we ship to them is the same image that powers our web APIs. Why do we need a separate image for on-prem here?

BrendanWalsh commented 5 years ago

This commit should also remove the tgz under repo/speech-onprem/, right?

anthonsu commented 5 years ago

In general our contract with customers is that the image we ship to them is the same image that powers our web APIs. Why do we need a separate image for on-prem here?

The docker image I use is basically from this https://docs.microsoft.com/en-us/azure/cognitive-services/speech-service/speech-container-howto?branch=release-build-cogserv-speech-services And this helm chart is created with purpose that providing customer an idea of how to use deploy onpremise speech service container into k8s cluster. In future, this helm chart (or docker container itself) may be able to let customers to apply their own on premises speech models (or anything else, I don't know yet). So I'd suggest keeping onpremise key word here.

anthonsu commented 5 years ago

This commit should also remove the tgz under repo/speech-onprem/, right?

Yes, it doesn't? Let me verify. Good catch :)

anthonsu commented 5 years ago

Ah, it doesn't remove speech-onprem because I synced up with master branch instead of gh-pages. Let me update.

anthonsu commented 5 years ago

./speech-onprem stuff is removed now :)

dbanda commented 5 years ago

Just a tiny comment. Can you also add an image for the chart if you have one?