[Gerrrr]

This PR contains my understanding of what we want to achieve in #118.

Changes:

• regressions command is removed as a Datastax-specific thing. analyze covers all interesting use cases.
• analyze --branch is ConfigArgParsified and can be configured via BRANCH env variable.
• Configuration syntax for analyze --branch is unified as much as possible across importers. As a standard, I used Graphite %BRANCH syntax. It is reused it in Postgres and BigQuery importers plus. Csv importer has its own thing. It looks at a branch column and behaves in the following way:
  1. If there is no branch column and --branch is not specified, it works as before.
  2. If there is a branch column and --branch is not specified, the importer checks that that column contains a unique value a guardrail against mixing results between different branches.
  3. If there is a branch column and --branch is specified, the importer will filter out values for rows where the value in branch column is not equal to the passed value.
  4. If there is no branch column and --branch is specified, the importer throws.
• There is no default value for --branch now and a test config has at least one %BRANCH, a user must pass it.
• %{BRANCH} is escaped against SQL injections in PostgreSQL and BigQuery, just in case.

[Gerrrr]

Bump @henrikingo :)

[henrikingo]

+1

Thanks Alex. I think this is as discussed in #118 and with these changes I'm good to go for a next release. Two comments just to highlight where there's future work:

This is actualy a fairly superficial change, yet it ends up being hundreds of lines changed. Ok, so half of that is test coverage, which is good, but the other half is ust because we copypasted the same code around each new importer. This aspect of the code deserves a serious refactoring. But let's release this first. The new e-divisive implementation deserves to get out there!

What's the difference between --branch and --attrs branch? The truth is, there's no need for a separate --branch option, it's just a dimension/label like many other attributes. (repository url, operating system, etc...)

[Gerrrr]

This aspect of the code deserves a serious refactoring.

+1, I plan to refactor the importer code soon as a step towards #55.

What's the difference between --branch and --attrs branch

hmmm, I did not realize one can pass attributes via CLI. Let me experiment with it; we may not need --branch after all.

[Gerrrr]

What's the difference between --branch and --attrs branch

--attrs only works with GraphiteImporter. Also, attrs does not allow you to overwrite a parameter from CLI - it just filters out attributes that are not explicitly specified -

otava/otava/importer.py

Lines 163 to 181 in 2bb687f

	run_ids = []
	commits = []
	versions = []
	branches = []
	for t in time:
	event = events_by_time.get(t)
	run_ids.append(event.run_id if event is not None else None)
	commits.append(event.commit if event is not None else None)
	versions.append(event.version if event is not None else None)
	branches.append(event.branch if event is not None else None)
	attributes = {
	"run": run_ids,
	"branch": branches,
	"version": versions,
	"commit": commits,
	}
	if selector.attributes is not None:
	attributes = {a: attributes[a] for a in selector.attributes}

.

Additionally, this attribute list is Fallout-specific with run_id and version there.

[Gerrrr]

On a slightly different topic, after the conversation in #127 I am having second thoughts about only doing string replacement with --branch and not addressing "A/B" testing in Otava CLI in any way. I will share an idea in #118 in order not to block this PR.