Re: [I] Fix branch interpolation logic in regressions command [otava]

["Gerrrr (via GitHub)" <gi...@apache.org>]

Gerrrr commented on issue #118:
URL: https://github.com/apache/otava/issues/118#issuecomment-3735760450

   Hey @henrikingo ! Sorry for the long reply. I am finally back from Christmas/New Year holidays and caught up with work.
   
   > I actually had not realized you can put entire sql statements in the yaml files. This must have been added when you added pgsql support? 
   
   Yes, this is helpful in cases, where the same benchmark runs with different configurations, multiple environments environments (e.g, with in-memory vs on-disk storage, on different instances, etc), and against multiple branches (e.g., all supported Kafka versions).
   
   > I would be willing to propose we remove the regressions command entirely, including the --branch option.
   
   +1 to removing `regressions` command. 
   
   To clarify, what do you think about ConfigArgParsed `--branch BRANCH` in the `analyze` command? We already support `analyze --branch`, but it can be configured only with a CLI argument.  While I 100% agree now that we do not really need any special functionality to support feature branch analysis (as we do not want to do any extra comparisons with the base branch), it is almost always helpful in my cases to pass the branch name to a test, because the same experiment usually runs against many branches.
   
   > IMO it would be good if this can be split into a completely separate file, so that the config that is managed with ConfigArgParse is clearly separate and also "just works" without needing to hack ConfigArgParse.
   
   Can you please elaborate on this? What would you put into the main config vs new separate files? At this point, it does not look like we need to hack ConfigArgParse any longer; all we need to do to satisfy the initial motivation for this ticket is to:
   1. Remove `regressions` command.
   2. ConfigArgParsify `--branch` in `analyze` command.
   
   > I still feel this is re-inventing a standard yaml feature (anchors)
   
   Hah, I did not know YAML achors exist. +1 replacing templates with acnhors. Let's create a ticket .


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@otava.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org