search

pachyderm / helmchart

Helm Chart for Pachyderm
5 stars 9 forks source link

Disable postgres #131

Closed seslattery closed 3 years ago

CLAassistant commented 3 years ago

CLA assistant check
All committers have signed the CLA.

chainlink commented 3 years ago

I think this should be enabled to follow the convention of the rest of the services

chainlink commented 3 years ago

Looks good otherwise, might be good to have a quick test here as well

seslattery commented 3 years ago

I think this should be enabled to follow the convention of the rest of the services

So I went back and forth on this a bit. I think enabled makes sense if it's an optional dependency. Pachd has a hard requirement on a postgres instance existing, so I think by stating I am bringing my own external postgres, it is more clear what's expected. I'm happy to change it to enabled if you feel strongly, I just felt it was slightly more confusing.

chainlink commented 3 years ago

I know the language is awkward, but feel strongly it should match the other services with enabled (I'm also adding one to pachd itself for enterprise server deployments)

chainlink commented 3 years ago

Will close #127

chainlink commented 3 years ago

Looks good! Need to run make pachyderm/values.schema.json to update the json schema

seslattery commented 3 years ago

closing - covered in #189