Thanks and sorry I haven't had the opportunity to check up on it earlier. I guess this hand checking before voting process has its merits. Hopefully we learn to find and fix issues even before the vote. Eventually.
Reviewing my own email after a good nights sleep, I think the really broken behavior are 1, 2, 5 and out of those 2 is a documentation fix. For 3 I think it a) may need discussion if not everyone shares this goal, but even if we do, then b) the goal need not be achieved in 0.7, especially if it is in conflict with maintaining backward compatibility. 4 and 7 are kind of consequences of 3, and finally 6 is IMO wrong but rather benign/minor. ... I spent some time trying to understand what's going on... I think the last commit in https://github.com/apache/otava/pull/95 reveals that we've not been clear with what we are trying to do. I thought the expandvars was fixing something related to templates, which I never used so I probably glanced over it. Now that I re-read I realize you (Alex) are undoing the whole point of what pull/86 tried to do. OTOH #86 is at fault for not updating examples and docs... For example, if I pay more attention now to the tests added in: https://github.com/apache/otava/blob/5276f24de6f0853f8304374dd1b3772501863c3c/tests/config_test.py#L222 ...then #95 makes this work: # Test dictionary expansion test_dict = { "host": "${TEST_HOST}", "port": "${TEST_PORT}", "database": "${TEST_DB}", "connection_string": "postgresql://${TEST_USER}@ ${TEST_HOST}:${TEST_PORT}/${TEST_DB}", "timeout": 30, # non-string should remain unchanged "enabled": True, # non-string should remain unchanged } result_dict = expand_env_vars_recursive(test_dict) expected_dict = { "host": "localhost", "port": "8080", "database": "testdb", "connection_string": "postgresql://testuser@localhost :8080/testdb", "timeout": 30, "enabled": True, } However, the whole point of #86 was to no longer use a mix of config files that embed env vars... What you want to do above, can now be done in one of three ways: YAML, just like before but no env vars: postgresql: host: "localhost", port: "8080", database: "testdb", connection_string: "postgresql://testuser@localhost :8080/testdb", timeout: 30, enabled: True, Or, the same can be expressed now as cli arguments: --postgresql-host localhost \ --postgresql-port 8080 \ --postgresql-database testdb \ --postgresql-connection-string ... And as the equivalent env vars: OTAVA_POSTGRESQL_HOST=localhost OTAVA_POSTGRESQL_PORT=8080 .... (Didn't check the last ones but something like that is the structure.) So,if you prefer to define things as env vars you totally can, but you cannot put env vars in the config file and expect them to be substituted. Welcome discussion, I'm not trying to be authoritative, merely trying to understand what each of us is thinking... henrik On Tue, Nov 18, 2025 at 4:28 AM Alexander Sorokoumov < [email protected]> wrote: > Thanks for the thorough review, Henrik! I will get to fixing these issues > around next Tuesday if no one beats me to it. > > FWIW let's keep using this thread to discuss issues 1-7 or if we find > anything else in this RC in the meantime. > > Best, > Alex > > On Sun, Nov 16, 2025 at 1: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
