After coming back from vacation, attempting to fix listed issues, and reflecting a bit more on them, I can't help, but propose to revert https://github.com/apache/otava/commit/69d2b97a6c38873b74755bf1104a69f099b922b9 as well as my humble attempts to fix bugs it introduced - https://github.com/apache/otava/commit/484aaef8493a076b42d1b0e92679d9edb87fb043, https://github.com/apache/otava/pull/95/commits/0644e7f31b5b53109fdc625114684735aa33e865 , <https://github.com/apache/otava/pull/95/commits/0644e7f31b5b53109fdc625114684735aa33e865,> etc. This will fix all problems except #2.
I want to revert it for 3 reasons: 1. As we keep finding new issues right before the release, it seems that our tests do not cover CLI and e2e usage well enough. Hence, I would not be comfortable to release changes to argument parsing logic at least without adding those tests. 2. This work is incomplete, as it does not fully replace expandvars functionality. One example is being able to specify the branch in PostgreSQL and BigQuery examples ( https://github.com/apache/otava/blob/5276f24de6f0853f8304374dd1b3772501863c3c/examples/postgresql/config/otava.yaml#L88) as `regressions --branch` CLI argument covers the feature branch, not the base branch. Fixing that will require changing `regressions` arguments, which will break backwards compatibility. 3. This patch effectively broke master and I would like to set the precedent that it is okay to revert commits that do not meet the quality bar. I propose the following: 1. Revert changes related to CLI-parsing. 2. Do 0.7.0 release - the last planned minor release in the major version 0, where we guarantee backwards compatibility with Hunter. After that, if there is demand, we _may_ offer patch releases by creating a branch "0.7.x" from the release commit. 3. Start working on features for next major release - 1.0.0, including: 3.1. Replace signal-processing-algorithms - merge https://github.com/apache/otava/pull/96. 3.2. Add extensive tests covering documented examples and CLI commands. 3.3. Upgrade to supported Python versions. 3.4. Change `regressions --branch` such that one can specify both base and feature branches. Maybe replace it with --base-branch and --feature-branch arguments? 3.4. Re-introduce ConfigArgParse. Re: > However, the whole point of #86 was to no longer use a mix of config files that embed env vars.. I hope that the reason #2 why I want to revert ConfigArgParse answers this concern. I think it is a noble goal to replace env variables with CLI arguments, but I do not see a way to do that without breaking existing workflows, hence I'd rather do it in the next major release. Best, Alex On Sun, Nov 23, 2025 at 9:57 AM Henrik Ingo <[email protected]> wrote: > Thanks Dave! > > I added it to the same PR. I think it's good to have a few different > scripts to demonstrate and hopefully benefit from a commitment to a certain > level of heterogenity in the verification and voting process. > > henrik > > On Sun, Nov 16, 2025 at 10:04 PM Dave Fisher <[email protected]> wrote: > > > Here’s a script I use to visually check downloads, signatures, and > > checksums: > > > > #!/bin/bash > > > > export DISTURL='https://dist.apache.org/repos/dist/dev' > > export PROJECT=${1} > > export ARTIFACT=${2} > > export DISTRO=${DISTURL}/${PROJECT}/${ARTIFACT} > > > > echo ${DISTRO} > > > > export TMPDIR=/tmp/${PROJECT} > > > > mkdir -p $TMPDIR > > cd $TMPDIR > > pwd > > > > curl -f -L ${DISTRO} --output ${ARTIFACT} > > curl -f -L ${DISTRO}.asc --output ${ARTIFACT}.asc > > curl -f -L ${DISTRO}.sha256 --output ${ARTIFACT}.sha256 > > curl -f -L ${DISTRO}.sha512 --output ${ARTIFACT}.sha512 > > > > echo 'Check signature' > > gpg --verify ${ARTIFACT}.asc > > echo 'Compare checksum to sha256' > > cat ${ARTIFACT}.sha256 > > shasum -a 256 ${ARTIFACT} > > echo 'Compare checksum to sha512' > > cat ${ARTIFACT}.sha512 > > shasum -a 512 ${ARTIFACT} > > echo > > > > > > Best, > > Dave > > > > > On Nov 16, 2025, at 11:38 AM, Henrik Ingo <[email protected]> wrote: > > > > > > Thank you Alex > > > > > > It's definitely getting better, but I ended up still voting -1, mainly > > due > > > to the -h regression. Which is first in the list below. > > > > > > Thanks to your changing the version strings in the URL to match, I > > > continued to develop a personal script that can be used to verify the > > > download, signature and functionality of the tar archive. Since there > was > > > discussion earlier that such scripts can be shared, I've done so in a > PR: > > > https://github.com/apache/otava/pull/98 I can also make it into a > gist, > > if > > > we don't want to keep it in the repository. If we decide to keep it in > > the > > > repository, I would encourage others to share similar scripts too. It > > seems > > > against the spirit of the ASF release process to have one "official" > > script > > > only. > > > > > > Issues with rc3: > > > > > > 1) > > > > > > otava -h > > > > > > raises NotImplementedError. This is a regression compared to 0.6.1 > > release. > > > I think it happens at 484aaef8493a076b42d1b0e92679d9edb87fb043 although > > > reading the diff I don't quite see why, and would find it easier to > > believe > > > if it was the previous commit. Our introduction of our > > > own NestedYAMLConfigFileParser subclass in any case seems to be the > > source > > > of this issue. > > > > > > I'll also note the plain `otava` still works, and also something like > > > `otava analyze -h` works. But I didn't find that we would have added a > > > pytest for either of these. So please add for all three. > > > > > > > > > > > > 2) > > > > > > GETTING_STARTED.md still refers to and essentially requires the file > > > `resources/otava.yml` but a directory called resources does not exist > in > > > the tar file nor in the git repo. > > > > > > > > > > > > The following I would not have considered alone as blocking issues, but > > > listing them here for completeness. > > > > > > 3) > > > Related to #2, I was hoping the introduction of ConfigArgParse would > > bring > > > us to a place where you could execute simple examples of otava without > > > needing a config file at all. In practice this could be supported for > the > > > CSV type, so that you don't depend on a database connection before you > > can > > > test otava the first time. We seem to not be quite there yet, as below > > > examples demonstrate. But I wanted to spell this out explicitly so that > > > others can either disagree or help achieve it. I realize historically > > > Hunter did never work without a config file. > > > > > > 4) > > > Specifically, 484aaef8493a076b42d1b0e92679d9edb87fb043 seems to omit > CSV > > > data type completely, so that I can no longer provide CSV related > options > > > from the command line. Hence this fails because the --test option is > > > ignored: > > > > > > uv run otava --config-file examples/csv/config/otava.yaml > > > --tests-local.sample-file=examples/csv/data/local_sample.csv analyze > > > local.sample > > > uv run otava --config-file examples/csv/config/otava.yaml analyze > > > --tests-local.sample-file=examples/csv/data/local_sample.csv > local.sample > > > > > > Acknowledging that I reviewed that commit myself... There seems to be > an > > > asymmetry here, where database connection parameters have config > options > > > like --postgres-hostname, yet there is no --csv-filename, rather the > > > filename is embedded in the definition of the test itself, and is > > therefore > > > ignored after this commit, because it is under --tests-* namespace. > > > > > > 5) > > > ArgParse / ConfigArgParse should also fail if I provide a command line > > > option (or yaml option, for that matter) that it doesn't recognize. The > > > above is silently ignored, the failure is then what happens later > because > > > it doesn't find the file /data/local.sample. (Which is why I tried to > > > provide another path via CLI.) > > > > > > 6) > > > otava then prints an error message, but doesn't exit(1), rather returns > > > with 0 as the return value. > > > > > > 7) > > > `otava analyze -h` doesn't print any of the database connection > options. > > So > > > it is left unclear whether they should be provided as `otava > > > --postgres-hostname... analyze` or `otava analyze --postgres` > > > > > > For reference, plain `otava` lists them and presumably `otava -h` would > > > explain them if it worked: > > > > > > usage: otava [-h] [--config-file CONFIG_FILE] [--graphite-url > > GRAPHITE_URL] > > > [--grafana-url GRAFANA_URL] [--grafana-user GRAFANA_USER] > > > [--grafana-password GRAFANA_PASSWORD] [--slack-token SLACK_TOKEN] > > > [--postgres-hostname POSTGRES_HOSTNAME] [--postgres-port > > > POSTGRES_PORT] [--postgres-username POSTGRES_USERNAME] > > [--postgres-password > > > POSTGRES_PASSWORD] [--postgres-database POSTGRES_DATABASE] > > > [--bigquery-project-id BIGQUERY_PROJECT_ID] > > > [--bigquery-dataset BIGQUERY_DATASET] [--bigquery-credentials > > > BIGQUERY_CREDENTIALS] > > > > > > > > > {list-tests,list-metrics,list-groups,analyze,regressions,remove-annotations,validate} > > > ... > > > > > > > > > > > > I'll end this email by saying that I think we agreed that 0.7.0 is > still > > > attempting to be a reasonably polished, but first and foremost a > backward > > > compatible release of the legacy Otava code, so it's still python 3.8, > > and > > > so on. Fixing above points 3-7 might very well require rethinking the > > > structure of the config files, so if that's the only way, then let's do > > it > > > after 0.7.0. After all, the above does work as documented. But it could > > be > > > a lot nicer. > > > > > > henrik > > > > > > > > > On Thu, Nov 13, 2025 at 9:40 AM Alexander Sorokoumov < > > > [email protected]> wrote: > > > > > >> Hello everyone, > > >> > > >> Please review and vote for the releasing Apache Otava > > 0.7.0-incubating-rc3. > > >> > > >> Changelog for this release candidate > > >> > > >> > > > https://github.com/apache/otava/compare/0.6.1-incubating...0.7.0-incubating-rc3 > > >> . > > >> The official Apache source release has been deployed to > > >> > > > https://dist.apache.org/repos/dist/dev/incubator/otava/0.7.0-incubating-rc3 > > >> . > > >> GH tag for release > > >> https://github.com/apache/otava/releases/tag/0.7.0-incubating-rc3. > > >> The artifacts have been signed with Key [ E81152E1F17593C0949A9D235E > > >> 2C934B6C5147A0 ], corresponding to [email protected] available > > here > > >> https://dist.apache.org/repos/dist/release/incubator/otava/KEYS. > > >> Please download, verify, and test. > > >> > > >> Please vote on releasing this candidate by replying with: > > >> [ ] +1 Release this package > > >> [ ] 0 No opinion > > >> [ ] -1 Do not release (please provide reason) > > >> > > >> To learn more about Apache Otava, please see https://otava.apache.org > . > > >> > > >> This vote will be open for at least 72 hours. > > >> > > >> Checklist for reference: > > >> [ ] Download links are valid. > > >> [ ] Checksums and signatures. > > >> [ ] LICENSE/NOTICE files exist. > > >> [ ] No unexpected binary files. > > >> [ ] All source files have ASF headers. > > >> [ ] Can install from source. > > >> [ ] Can run examples using all supported Python versions. > > >> > > >> Best, > > >> Alex > > >> > > > > > > > > > -- > > > *nyrkio.com <http://nyrkio.com/>* ~ *git blame for performance* > > > > > > Henrik Ingo, CEO > > > [email protected] LinkedIn: > > > www.linkedin.com/in/heingo > > > +358 40 569 7354 Twitter: > > twitter.com/h_ingo > > > > > > -- > *nyrkio.com <http://nyrkio.com/>* ~ *git blame for performance* > > Henrik Ingo, CEO > [email protected] LinkedIn: > www.linkedin.com/in/heingo > +358 40 569 7354 Twitter: > twitter.com/h_ingo >
