feat!: bump hedgedoc version to 1.10.7, move to cloudpirates chart#1695
feat!: bump hedgedoc version to 1.10.7, move to cloudpirates chart#1695
Conversation
88cbe10 to
2b171e3
Compare
2b171e3 to
d01d920
Compare
|
It looks like there are some outdated references to bitnami postgresql: These will most likely need to be addressed before we can get this merged. |
|
@hairmare what's the way to go here? We originally started shipping this with a bitnami DB because of the philosophy that every chart should install out of the box (and during linting). Pivoting away from Bitnami for a minor release would probably be too breaking. |
|
let's ask @c0rydoras how he solved it for the timed chart 😬 |
|
that said,,,, pivoting from bitnami is ok, we might want to mention that the database isn't considered to be part of any breaking api, we recommend that people care about their database and don't manage it via the included chart. The included setup enables PoC work + CI, but it ain't your free ticket to a dba managed env. |
charts/hedgedoc/Chart.yaml
Outdated
| type: application | ||
| version: 0.5.4 | ||
| appVersion: "1.10.3" | ||
| version: 0.5.5 |
There was a problem hiding this comment.
As mentioned earlier, even if this should not be used for production workloads it could still introduce a breaking change depending on the values used. So I would at least do a minor version bump, instead of just a patch.
There was a problem hiding this comment.
I thought I upped this to 1.0.0 o_O
this is 100% breaking
-> will update
There was a problem hiding this comment.
I agree that we should stick to semver and even do a major upgrade, but I'll let my colleagues some time to review this too!
IIRC the bitnami chart doesn't set the retention policy to Retain by default, so this could potentially lead to data loss if not investigated correctly on improper setup environments. But still, this is a breaking change, as it also requires changing the values due to the changed value set coming from the dependency.
The only alternative for me would be to remove the database from the dependency completely, either we offer it and handle the weight of it, or we remove it and leave it to the users to bring a DB.
There was a problem hiding this comment.
Sticking to semver includes being allowed to introduce breaking changes at anytime when still using 0.x. I strongly recommend sticking with 0.x for now, if someone requires a 1.x then we would need to put a bit of work into clarifying what it includes.
In this case, the included PostgreSQL should have never been considered to be part of the external API surface, so what we do with it is never a breaking change.
Maybe we should clearly mention that the included database is for CI only and not for production workloads. Maybe that isn't clear enough yet?
BREAKING CHANGE: data needs to manaually be migrated from the bitnami postgres to the cloudpirates postgres install if an external database is not used. Signed-off-by: Gian Klug <gian.klug@adfinis.com>
681b988 to
8997156
Compare
Co-authored-by: Lucas <116588+hairmare@users.noreply.github.com>
Description
Issues
Checklist
artifacthub.io/changesannotation inChart.yaml, check the example in the documentation.pre-commit rundocs/