Skip to content
This repository was archived by the owner on Aug 23, 2023. It is now read-only.

Extract CQL vars into toml templates #892

Closed
wants to merge 2 commits into from

Conversation

beorn-
Copy link
Contributor

@beorn- beorn- commented Apr 17, 2018

No description provided.

@beorn- beorn- force-pushed the use_template branch 4 times, most recently from e5047d4 to fcf1e27 Compare April 17, 2018 17:20
AND compression = {'sstable_compression': 'org.apache.cassandra.io.compress.LZ4Compressor'}
"""

[cassandra-idx]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

people may use cassandra for the store and not the index, or vice versa, so putting both in the same file seems confusing. hence my original suggestion to have a file for each index plugin and each store plugin

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alright. i thought having everything in a single file would make it easier to change things from an admin point of view.

"""

[cassandra-idx]
type = "scylladb"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does this type do, why is it needed and why is it set to scylladb in the cassandra file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nothing special, it was informative. Moreover the value here is an error during rebase i'll fix this

util/template.go Outdated
type StoreConfigDatabase struct {
Type string `toml:"type"`
Create_DB_template string `toml:"create_db_template"`
Create_Table_template string `toml:"create_table_template"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as I pointed out in my comment in #888 I don't think we should generalize that a store has 1 createDB and 1 createTable template, it depends on the store

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay i'll make it specific

@@ -63,8 +63,13 @@ func main() {
log.Fatal(4, "failed to create cql session for destination cassandra. %s", err)
}

// read key(space|table) information from templates
var tomlstring = util.FileToString(*cassandraTemplate)
var conf = util.TomlCassandraToVar(tomlstring)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the caller of util shouldn't need to know about toml specifics or about the 2 separate steps.
I suggest instead an api like:

tableSchema, err := util.ReadEntry(*cassandraTemplate, "table-schema")
if err != nil {
...
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense, i'll change this.

@beorn- beorn- force-pushed the use_template branch 5 times, most recently from 44a7a0c to 4386ce2 Compare April 23, 2018 15:39
@Dieterbe
Copy link
Contributor

Hey @beorn- , nice work.
I took your branch, added some cleanup commits, and added a new docker-dev-scylla stack which uses the new scylladb templates.
also i moved the templates to /usr/share/metrictank/examples by default.
let's resume this in #898

@Dieterbe Dieterbe closed this Apr 27, 2018
@beorn- beorn- deleted the use_template branch May 3, 2018 08:39
@beorn- beorn- restored the use_template branch May 3, 2018 08:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants