Conversation
guy-har
left a comment
guy-har
left a comment
There was a problem hiding this comment.
Few comments, overall looks
Mainly blocking because:
- I think we should use the next image version (which has many fixes) before enabling replicatoin
- Just validate replication is supported/recomended to work with more than replica
nit: can we move the replication files into a directory like done in mds?
What about rediness and liveness probes? maybe we should have a follow up issue
charts/lakefs/values.yaml
Outdated
| # https://docs.lakefs.io/latest/howto/mirroring/ | ||
| replication: | ||
| enabled: false | ||
| replicas: 1 |
There was a problem hiding this comment.
Is it recommended to have more than one replication replica? If not let's not make it configurable, if there is a warning, let's add it.
There was a problem hiding this comment.
I have no idea TBH, never tested it with more than one replica.
It's configurable- the user will have to override replication.replicas
There was a problem hiding this comment.
I think that increasing reources make more sense than increasing replica count here
| type: application | ||
| version: 1.7.20 | ||
| version: 1.7.21 | ||
| appVersion: 1.78.0 |
There was a problem hiding this comment.
Maybe we should wait for the next version, which should be released today/tomorrow. WDYT?
There was a problem hiding this comment.
I don't think it's necessary
I don't want to be a blocker for whoever is going to release a new version.
Replication app version can always be changed by the user or us
There was a problem hiding this comment.
We can set the default tag to be 0.1.17, which will be released tomorrow instead
| - name: REPLICATION_LAKEFS_ACCESS_KEY_ID | ||
| valueFrom: | ||
| secretKeyRef: | ||
| name: {{ .Values.replication.extraEnvVarsSecret }} | ||
| key: source_lakefs_access_key_id |
There was a problem hiding this comment.
Not sure this should a env var or a secret, but it's fine like this
|
2 participants
No description provided.