-
Notifications
You must be signed in to change notification settings - Fork 192
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
Add explicit authentication #17
Conversation
@@ -264,6 +264,7 @@ def main(): | |||
p.add_argument('-k', '--kibana_nested', dest='kibana_nested', action='store_true', help='Format nested fields in Kibana style.') | |||
p.add_argument('-r', '--raw_query', dest='raw_query', action='store_true', help='Switch query format in the Query DSL.') | |||
p.add_argument('-e', '--meta_fields', dest='meta_fields', action='store_true', help='Add meta-fields in output.') | |||
p.add_argument('-a', '--auth', dest='auth', type=str, required=False, help='Elastic search basic authentication in the form of username:password') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add dot (.) at the end of sentence and please add documentation to readme about this option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, thanks for considering the PR. I fixed the typos and added the option as well as an explicit example in your readme file.
Thanks,
Artur
@@ -264,7 +264,7 @@ def main(): | |||
p.add_argument('-k', '--kibana_nested', dest='kibana_nested', action='store_true', help='Format nested fields in Kibana style.') | |||
p.add_argument('-r', '--raw_query', dest='raw_query', action='store_true', help='Switch query format in the Query DSL.') | |||
p.add_argument('-e', '--meta_fields', dest='meta_fields', action='store_true', help='Add meta-fields in output.') | |||
p.add_argument('-a', '--auth', dest='auth', type=str, required=False, help='Elastic search basic authentication in the form of username:password') | |||
p.add_argument('-a', '--auth', dest='auth', type=str, required=False, help='Elasticsearch basic authentication in the form of username:password.') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please move this arg between -u
and -i
, it should be better place for authentication.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved in the middle
|
||
.. code-block:: bash | ||
|
||
$ es2csv -u http://my.cool.host.com:6666/es/ -q 'host: localhost' -o database.csv -a login:password |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please move -a login:password
in front of -u
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to the beginning before u
@@ -45,6 +45,7 @@ Usage | |||
-k, --kibana_nested Format nested fields in Kibana style. | |||
-r, --raw_query Switch query format in the Query DSL. | |||
-e, --meta_fields Add meta-fields in output. | |||
-a, --auth Elasticsearch basic authentication in the form of username:password. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-u, --url URL Elasticsearch host URL. Default is http://localhost:9200.
-a, --auth AUTH Elasticsearch basic authentication in the form of username:password.
-i, --index-prefixes INDEX [INDEX ...] Index name prefix(es). Default is ['logstash-*'].
And please fix header:
$ es2csv.py [-h] -q QUERY [-u URL] [-a AUTH] [-i INDEX [INDEX ...]]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added header args descirption
Good job! |
Yey :) No problem! Do you have an estimate on when you would release it? |
Hi,
I created this pull request because I ran into the following issue, documented here:
elastic/elasticsearch-py#470 (comment)
It appears that python does not like the have hash signs in passwords when using the recommended RFC-1738. So, for example, a password like "myPassword#" will break authentication with ES.
This PR adds the following:
Elasticsearch will prefer RFC-style auth when both are specified. I could be convinced to add validation for this case (where both are specified). This version does not have that as I saw it unnecessary for my purposes :)
Let me know your thoughts!
-- Artur