Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove deprecated --setup, --version, and --configflags CLI flags #10138

Merged
merged 15 commits into from
Jan 30, 2019

Conversation

urso
Copy link

@urso urso commented Jan 17, 2019

Remove flags and references from:

  • libbeat/cmd/instance
  • tests
  • Beats Documentation

@urso urso added the review label Jan 17, 2019
@urso urso requested review from kvch and dedemorton January 17, 2019 00:37
@urso urso requested a review from a team as a code owner January 17, 2019 00:37
@urso urso force-pushed the remove-setup-flag branch from 883996d to e51c7d9 Compare January 19, 2019 15:13
@urso urso changed the title Remove deprecated --setup and --version CLI flags Remove deprecated --setup, --version, and --configflags CLI flags Jan 19, 2019
@kvch
Copy link
Contributor

kvch commented Jan 21, 2019

The package libbeat still contains a reference to -setup flag in libbeat/_meta/config.yml under the dashboards section:

Loading the dashboards is disabled by default and can be enabled either by setting the
options here, or by using the -setup CLI flag or the setup command.

I have found a few instaces of -configtest in the folllowing E2E tests:

  • packetbeat/tests/system/test_0026_test_config.py
  • winlogbeat/tests/system/test_config.py
  • winlogbeat/tests/system/test_eventlogging.py
  • winlogbeat/tests/system/test_wineventlog.py

Nit: One more comment referring to -version in libbeat/cmd/instace/beat.go:

 // handleFlags parses the command line flags. It handles the '-version' flag
// and invokes the HandleFlags callback if implemented by the Beat.
func (b *Beat) handleFlags() error {

@urso
Copy link
Author

urso commented Jan 21, 2019

@kvch Thanks. These settings are everywhere :)

@urso urso requested review from a team as code owners January 22, 2019 20:26
@urso
Copy link
Author

urso commented Jan 24, 2019

beats-ci is acting up once again :(

Travis is green + related windows tests have been green as well: https://beats-ci.elastic.co/job/elastic+beats+pull-request+multijob-windows/beat=winlogbeat,label=windows/3948/console

@urso urso added the libbeat label Jan 24, 2019
@cwurm cwurm removed the request for review from a team January 24, 2019 11:07
@urso
Copy link
Author

urso commented Jan 24, 2019

Jenkins, test this.

@urso urso force-pushed the remove-setup-flag branch from 59edd77 to 59301d1 Compare January 25, 2019 13:09
-----

Or:

["source","sh",subs="attributes"]
-----
{beatname_lc} -e --setup
{beatname_lc} -e
Copy link
Contributor

Choose a reason for hiding this comment

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

Also remove the sentence on line 571/572 that says: "Use this command instead of run --setup when you want to set up the environment without actually running {beatname_uc} and ingesting data.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks. I modified the sentence to:

Use this command if you want to set up the environment without actually running
{beatname_uc} and ingesting data.

@urso urso force-pushed the remove-setup-flag branch from 59301d1 to ec12eed Compare January 30, 2019 13:17
@urso urso removed the request for review from a team January 30, 2019 14:53
Copy link
Contributor

@dedemorton dedemorton left a comment

Choose a reason for hiding this comment

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

Doc changes LGTM

@urso urso merged commit 69fba5f into elastic:master Jan 30, 2019
@urso urso deleted the remove-setup-flag branch February 19, 2019 18:29
DStape pushed a commit to DStape/beats that referenced this pull request Aug 20, 2019
…astic#10138)

* Remove deprecated --setup and --version, and -configtest flags
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants