Skip to content

Conversation

@Gerrrr
Copy link
Contributor

@Gerrrr Gerrrr commented Nov 11, 2025

  1. Fix incorrect docker run commands in examples, identified in the release vote thread - https://lists.apache.org/thread/35rvd63kcqdzdsw0hmfs087dnvgkpv1t.
  2. Bump year in Postgres examples.
  3. Fixup env variable expansion. Before OTAVA-82: use ConfigArgParse to create Config #86 Otava replaced all env variables; 0644e7f brings this behavior back.

Kudos to @Sowiks who noticed issues with PostgreSQL example during release vote.

By default, Otava looks 1 year back. Example data is too old.
This commit fixes behavior removed in #86 - expand all env variables,
not just ones matching CLI arguments. There are users relying on this
behavior so we should not break it without longer discussion and a
major release.
@Gerrrr Gerrrr changed the title Fix docker run commands in examples Fix multiple issues discovered via running PostgreSQl example Nov 11, 2025
@Gerrrr Gerrrr changed the title Fix multiple issues discovered via running PostgreSQl example Fix multiple issues discovered via running examples Nov 11, 2025
Copy link
Contributor

@henrikingo henrikingo left a comment

Choose a reason for hiding this comment

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

How did we not have expandvars? Was that the bug that we accidentally dropped it?

@Gerrrr
Copy link
Contributor Author

Gerrrr commented Nov 11, 2025

My interpretation is that #86 assumed that we only ever want to have env vars that match CLI args, which is not the case.

@Sowiks
Copy link
Contributor

Sowiks commented Nov 11, 2025

Some examples still don't work for me (see below)

Master branch:

  • CSV example works
  • POSTGRESQL, GRAPHITE, GRAPHANA
  • incorrect commands in .md example files for POSTGRESQL, GRAPHITE, GRAPHANA (extra otava's)

PR-95 (this one):

  • CSV, POSTGRESQL examples works
  • GRAPHITE, GRAPHANA examples do not work (No timeseries found in Graphite for test my-product.test. - same error as on master branch)
  • all commands in .md example files are correct

Release Apache Otava 0.7.0-incubating-rc2 (see https://lists.apache.org/thread/2dr1ccy9yb278wgxy611wz5p2ypqjhnv)

  • CSV, GRAPHITE, GRAPHANA examples works
  • POSTGRESQL examples do not work (LOG: unexpected EOF on client connection with an open transaction - same error as on master branch)
  • incorrect commands in .md example files for POSTGRESQL, GRAPHITE, GRAPHANA (same as on master branch)

I would suggest implement the same changes for GRAPHITE, GRAPHANA examples as in 0.7.0-incubating-rc2 here.

@Sowiks
Copy link
Contributor

Sowiks commented Nov 12, 2025

Update: I figured out that the problem was on my side due to me building the docker container on windows. Git clone replaced line endings symbols with windows-based ones which messed with docker. git config --global core.autocrlf false solved the problem.

@Gerrrr Gerrrr merged commit 5276f24 into master Nov 13, 2025
4 checks passed
henrikingo pushed a commit that referenced this pull request Nov 16, 2025
* Fix docker run commands in examples

* Bump year in the postgres example data

By default, Otava looks 1 year back. Example data is too old.

* Expand all env variables and fix PostgreSQL example

This commit fixes behavior removed in #86 - expand all env variables,
not just ones matching CLI arguments. There are users relying on this
behavior so we should not break it without longer discussion and a
major release.
@henrikingo
Copy link
Contributor

My interpretation is that #86 assumed that we only ever want to have env vars that match CLI args, which is not the case.

That was certainly my intent with #86, or generally the typical use of ConfigArgParse. Why would we not want them to be the same?

Gerrrr added a commit that referenced this pull request Nov 28, 2025
Gerrrr added a commit that referenced this pull request Nov 28, 2025
* Revert "Fix multiple issues discovered via running examples (#95)"

This reverts commit 5276f24.

* Revert "Fix config parsing (#91)"

This reverts commit 484aaef.

* Revert "OTAVA-82: use ConfigArgParse to create Config (#86)"

This reverts commit 69d2b97.

* Fix docker run commands in examples

* Bump year in the postgres example data

By default, Otava looks 1 year back. Example data is too old.

* Update CLI help description

* Add e2e tests covering CSV and PostgreSQL usage

* OTAVA-65: Fix --help requiring a config file

* Run tests against branch 0.7

* Fixup csv docs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants