Skip to content

Conversation

@marksilcox
Copy link
Contributor

Motivation

Without a new line at the end of the file, running apply-config-from-env-with-prefix.py with a new property appends directly to the file so you end up with something like managedLedgerMaxUnackedRangesToPersistInZooKeeper=-1myotherproperty=value and startup fails

Modifications

Added a new line at end of file

Verifying this change

  • Make sure that the change passes the CI checks.

This change is a trivial rework / code cleanup without any test coverage.

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): ( no)
  • The public API: ( no)
  • The schema: ( no
  • The default values of configurations: ( no)
  • The wire protocol: ( no)
  • The rest endpoints: (no)
  • The admin cli options: (no)
  • Anything that affects deployment: ( no)

Documentation

Need to update docs?

  • no-need-doc
    simple conf file change

@Technoboy- Technoboy- changed the title [bug][docker] Added newline to end of standalone.conf [fix][broker] Added newline to end of standalone.conf May 13, 2022
@Technoboy- Technoboy- added type/bug The PR fixed a bug or issue reported a bug area/broker doc-not-needed Your PR changes do not impact docs labels May 13, 2022
@Technoboy- Technoboy- added this to the 2.11.0 milestone May 13, 2022
Copy link
Contributor

@merlimat merlimat left a comment

Choose a reason for hiding this comment

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

👍

In any case, the script was also already fixed to handle it:

# Ensure we have a new-line at the end of the file, to avoid issue
# when appending more lines to the config
lines.append('\n')

@marksilcox
Copy link
Contributor Author

+1

In any case, the script was also already fixed to handle it:

# Ensure we have a new-line at the end of the file, to avoid issue
# when appending more lines to the config
lines.append('\n')

that is missing from apply-config-from-env-with-prefix.py

@merlimat
Copy link
Contributor

that is missing from apply-config-from-env-with-prefix.py

Oh, gotcha. Do you mind creating a PR to fix it there too? :)

@marksilcox
Copy link
Contributor Author

Oh, gotcha. Do you mind creating a PR to fix it there too? :)

No probs, created -> #15596

@Technoboy- Technoboy- merged commit 1c17f6d into apache:master May 14, 2022
@marksilcox marksilcox deleted the standalone_conf_fix branch August 31, 2022 07:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/broker doc-not-needed Your PR changes do not impact docs type/bug The PR fixed a bug or issue reported a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants