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

Allow specifying Kibana URL for mage ExportDashboard #10145

Merged
merged 4 commits into from
Jan 25, 2019

Conversation

cwurm
Copy link
Contributor

@cwurm cwurm commented Jan 17, 2019

When running Kibana from source (maybe also in other cases) Kibana will run under a URL like http://localhost:5603/kva (note the kva at the end).

This changes the mage ExportDashboard command to take into account a KIBANA_URL environment variable that allows specifying a custom Kibana URL like that to allow exporting dashboards.

Also improves a log message.

@cwurm cwurm requested a review from a team as a code owner January 17, 2019 14:26
libbeat/kibana/client.go Show resolved Hide resolved
dev-tools/mage/dashboard.go Show resolved Hide resolved
dev-tools/mage/dashboard.go Outdated Show resolved Hide resolved
@@ -36,6 +36,8 @@ func ExportDashboard() error {
return fmt.Errorf("Dashboad ID must be specified")
}

kibanaURL := EnvOr("KIBANA", "")
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we call this KIBANA_BASE_PATH because to remove it in the kibana build you need to run --no-base-path flag.

Copy link
Member

Choose a reason for hiding this comment

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

I prefer KIBANA_URL. That's what I used in the import target in the PR.

https://github.com/elastic/beats/pull/9842/files#diff-81a661bb38d6e9e450c732a07bdb4ac0R63

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm fine with either.

Copy link
Contributor

Choose a reason for hiding this comment

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

me too, KIBANA_URL then? :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good :)

@cwurm
Copy link
Contributor Author

cwurm commented Jan 24, 2019

I've changed to KIBANA_URL as discussed, can you have another look @ruflin @andrewkroh?

@ruflin
Copy link
Contributor

ruflin commented Jan 24, 2019

jenkins, test this

Copy link
Contributor

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

LGTM. Didn't test it locally.

@cwurm cwurm merged commit 2bb0a34 into elastic:master Jan 25, 2019
@cwurm cwurm deleted the export_dashboard_kibana_url branch January 25, 2019 10:06
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.

4 participants