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

Fix unsupported characters in Dashboards #2946

Merged
merged 5 commits into from
Nov 9, 2016

Conversation

martinscholz83
Copy link
Contributor

Fix #2940

@elasticsearch-release
Copy link

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run.

1 similar comment
@elasticsearch-release
Copy link

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run.

@karmi
Copy link

karmi commented Nov 7, 2016

Hi @maddin2016, we have found your signature in our records, but it seems like you have signed with a different e-mail than the one used in yout Git commit. Can you please add both of these e-mails into your Github profile (they can be hidden), so we can match your e-mails to your Github profile?

@martinscholz83
Copy link
Contributor Author

@andrewkroh, how can i test if export_dashboards.py is now working correctly?

@martinscholz83
Copy link
Contributor Author

@karmi, i only have this one. I also have confirmed the agreement per email. Then there is maybe a spelling mistake. Is there a way to re confirm the agreement?

@martinscholz83
Copy link
Contributor Author

I have compare these adresses and it they are identical

@martinscholz83
Copy link
Contributor Author

image

image

@monicasarbu
Copy link
Contributor

@maddin2016 Thank you for the fix. To test that the export_dashboards.py is working correctly, I suggest to first import the Packetbeat dashboards, and then export them locally using the export-dashboards.py script, that you changed. To import the Packetbeat dashboards, please use the scripts/import_dashboards from the Packetbeat 5.0.0 package to import the latest available dashboards.

@martinscholz83
Copy link
Contributor Author

i suppose that there is no proxy switch for import_dashboards.exe?

@martinscholz83
Copy link
Contributor Author

how can i import them per hand?

@andrewkroh
Copy link
Member

@maddin2016 Good question regarding the proxy, I think you should be able to use the environment variables specified here to configure your proxy settings.

Otherwise you can download the zip through another means from https://artifacts.elastic.co/downloads/beats/beats-dashboards/beats-dashboards-5.0.0.zip and use the -file flag.

@martinscholz83
Copy link
Contributor Author

I have fix the regex and can confirm that the : character is replaced.

image

@martinscholz83
Copy link
Contributor Author

martinscholz83 commented Nov 7, 2016

How can i write a unit test. I see that there is a test beats/libbeat/tests/system/test_dashboard.py. Can i use this test or is there a better place.
(Sorry for that many questions 🙊 )

@monicasarbu
Copy link
Contributor

@maddin2016 Thank you for adding a test for exporting dashboards.

@monicasarbu monicasarbu merged commit c3d9910 into elastic:master Nov 9, 2016
@ruflin
Copy link
Contributor

ruflin commented Nov 10, 2016

@maddin2016 @monicasarbu I opened this follow up PR here: #2979 Currently our tests are failing because of this change. I think the test needs some more improvements (see my comments in the PR).

@martinscholz83
Copy link
Contributor Author

@ruflin, that's a lot of new stuff for me. So thanks for your tips!! Do you merge #2979 so i can create a new PR to improve test?

@ruflin
Copy link
Contributor

ruflin commented Nov 10, 2016

@maddin2016 Yes, #2979 got just merged. No worries, we are here to help. Just ping us if you need some help / inputs.

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.

7 participants