Cassandra init #6072
Cassandra init #6072
Conversation
Signed-off-by: mehul <[email protected]>
Signed-off-by: mehul <[email protected]>
Signed-off-by: mehul <[email protected]>
Signed-off-by: mehul <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6072 +/- ##
==========================================
- Coverage 96.92% 96.72% -0.20%
==========================================
Files 349 350 +1
Lines 16598 16729 +131
==========================================
+ Hits 16087 16181 +94
- Misses 328 355 +27
- Partials 183 193 +10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| } | ||
| var datacentre, replications string | ||
| // var ReplicationFactor int | ||
| if mode == "prod" { |
There was a problem hiding this comment.
I don't think we need this distinction anymore. Users can provide precise values for parameters, we don't have to have Jaeger guess those parameters based on the MODE
There was a problem hiding this comment.
so should i remove the mode attribute and take prod mode as default
?
There was a problem hiding this comment.
yes. Whichever parameters the prod attribute affects today should simply be exposed as configuration options for the user.
There was a problem hiding this comment.
btw, we do not need to enable this schema-management capability in jaeger-v1, especially if we need new configuration for it - we can make it only configurable in v2 (which is easier, just define the struct with mapstructure tags)
| builder := &MappingBuilder{} | ||
| schema, _, err := builder.GetSpanServiceMappings() |
There was a problem hiding this comment.
where does the term "mapping" come from? If you're copying it from Elasticsearch, there "mapping" is actually a term understood by ES. No such thing in Cassandra.
There was a problem hiding this comment.
should i call it template or cassandra_template ?
| case "4": | ||
| template = "./v004.cql.tmpl" | ||
| default: | ||
| template = "./v004.cql.tmpl" |
There was a problem hiding this comment.
I am not sure how you are planning to use those files. They use variable placeholders like ${keyspace}, which works in shell substitution, but will not work as a Go template. We should be using Go template the this functionality. Your could do a search/replace, something like ${keyspace} ==> {{ .Keyspace }}. But it would be easier just to copy and change the syntax. You might need to use conditional clauses, like {{- if .UseILM}} from ES templates.
There was a problem hiding this comment.
this cqlOutput provides the same exact output as create.sh it is done in the RenderCQLTemplate function
Which problem is this PR solving?
Description of the changes
How was this change tested?
Checklist
jaeger:make lint testjaeger-ui:yarn lintandyarn test