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

Issue with unquoted csv output format #46

Closed
john-mcnamara-intel opened this issue Sep 12, 2019 · 12 comments
Closed

Issue with unquoted csv output format #46

john-mcnamara-intel opened this issue Sep 12, 2019 · 12 comments
Labels

Comments

@john-mcnamara-intel
Copy link

john-mcnamara-intel commented Sep 12, 2019

Some output fields from git-pw contain commas which break the CSVoutput.

For example consider a query like this against the DPDK Patchwork:

$ git-pw patch list RFC --column ID --column Name --limit 4
+-------+-----------------------------------------------------------------+
|    ID | Name                                                            |
|-------+-----------------------------------------------------------------|
| 59107 | [RFC] kni: properly translate pa2va for cloned mbuf             |
| 59106 | [RFC,v3,4/4] app/testpmd: show the Rx/Tx burst mode description |
| 59105 | [RFC,v3,3/4] net/ice: support to get the Rx/Tx burst mode       |
| 59104 | [RFC,v3,2/4] net/i40e: support to get the Rx/Tx burst mode      |
+-------+-----------------------------------------------------------------+

If you then add the format csv option you get this:

$ git-pw patch list RFC --column ID --column Name --limit 4 -f csv
ID,Name
59107,[RFC] kni: properly translate pa2va for cloned mbuf
59106,[RFC,v3,4/4] app/testpmd: show the Rx/Tx burst mode description
59105,[RFC,v3,3/4] net/ice: support to get the Rx/Tx burst mode
59104,[RFC,v3,2/4] net/i40e: support to get the Rx/Tx burst mode

The commas in the Name fields means that there are too many CSV fields. I'd guess that they should be quoted:

$ git-pw patch list RFC --column ID --column Name --limit 4 -f csv
ID,Name
59107,"[RFC] kni: properly translate pa2va for cloned mbuf"
59106,"[RFC,v3,4/4] app/testpmd: show the Rx/Tx burst mode description"
59105,"[RFC,v3,3/4] net/ice: support to get the Rx/Tx burst mode"
59104,"[RFC,v3,2/4] net/i40e: support to get the Rx/Tx burst mode"

Version is 1.6.0 from the current HEAD:

$ git-pw --version
git-pw, version 1.6.0
@john-mcnamara-intel
Copy link
Author

P.S. Simple quoting of the fields may not work either since the fields also include quotes. It is probably worth using csv.py for the task.

@stephenfin
Copy link
Member

csv.py? I was thinking we could make the separator configurable but that might not help much either. It's a tricky one. Open to suggestions.

@john-mcnamara-intel
Copy link
Author

csv.py

Sorry for being unclear, I meant the standard lib csv.py: https://docs.python.org/3/library/csv.html (although it may not fit in with your code).

@stephenfin
Copy link
Member

@john-mcnamara-intel I should have this fixed on master now. Can you test this and let me know if it fixes things for you? If so, I'll cut a new release.

Hint: to test locally without wiping your install:

# cd to project (DPDK?) directory
$ virtualenv .venv
$ source .venv/bin/activate
$ pip install https://github.com/getpatchwork/git-pw/archive/master.zip
# ... run your tests
$ deactivate
$ rm -rf .venv

@john-mcnamara-intel
Copy link
Author

john-mcnamara-intel commented Sep 16, 2019

Thanks for the fix.

With a full install in Python 2 the changes on HEAD give the following error:

$ git-pw patch list RFC --column ID --column Name --column Date --limit 4 -f csv
Traceback (most recent call last):
  File "/usr/bin/git-pw", line 10, in <module>
    sys.exit(cli())
  File "/usr/lib/python2.7/site-packages/click/core.py", line 716, in __call__
    return self.main(*args, **kwargs)
  File "/usr/lib/python2.7/site-packages/click/core.py", line 696, in main
    rv = self.invoke(ctx)
  File "/usr/lib/python2.7/site-packages/click/core.py", line 1060, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/usr/lib/python2.7/site-packages/click/core.py", line 1060, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/usr/lib/python2.7/site-packages/click/core.py", line 889, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/usr/lib/python2.7/site-packages/click/core.py", line 534, in invoke
    return callback(*args, **kwargs)
  File "/usr/lib/python2.7/site-packages/click/decorators.py", line 17, in new_func
    return f(get_current_context(), *args, **kwargs)
  File "/usr/lib/python2.7/site-packages/git_pw/api.py", line 303, in new_func
    return ctx.invoke(f, *args, **kwargs)
  File "/usr/lib/python2.7/site-packages/click/core.py", line 534, in invoke
    return callback(*args, **kwargs)
  File "/usr/lib/python2.7/site-packages/git_pw/patch.py", line 265, in list_cmd
    utils.echo_via_pager(output, headers, fmt=fmt)
  File "/usr/lib/python2.7/site-packages/git_pw/utils.py", line 138, in echo_via_pager
    output = _tabulate(output, headers, fmt)
  File "/usr/lib/python2.7/site-packages/git_pw/utils.py", line 73, in _tabulate
    result = StringIO()
NameError: global name 'StringIO' is not defined

Since the import probably should be:

if sys.version_info < (3, 0):
    from StringIO import StringIO  # noqa
else:
    from io import StringIO  # noqa

However, with that change in place it throws another issue:

$ git-pw patch list RFC --column ID --column Name --column Date --limit 4 -f csv
Traceback (most recent call last):
  File "/usr/bin/git-pw", line 10, in <module>
    sys.exit(cli())
  File "/usr/lib/python2.7/site-packages/click/core.py", line 716, in __call__
    return self.main(*args, **kwargs)
  File "/usr/lib/python2.7/site-packages/click/core.py", line 696, in main
    rv = self.invoke(ctx)
  File "/usr/lib/python2.7/site-packages/click/core.py", line 1060, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/usr/lib/python2.7/site-packages/click/core.py", line 1060, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/usr/lib/python2.7/site-packages/click/core.py", line 889, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/usr/lib/python2.7/site-packages/click/core.py", line 534, in invoke
    return callback(*args, **kwargs)
  File "/usr/lib/python2.7/site-packages/click/decorators.py", line 17, in new_func
    return f(get_current_context(), *args, **kwargs)
  File "/usr/lib/python2.7/site-packages/git_pw/api.py", line 303, in new_func
    return ctx.invoke(f, *args, **kwargs)
  File "/usr/lib/python2.7/site-packages/click/core.py", line 534, in invoke
    return callback(*args, **kwargs)
  File "/usr/lib/python2.7/site-packages/git_pw/patch.py", line 265, in list_cmd
    utils.echo_via_pager(output, headers, fmt=fmt)
  File "/usr/lib/python2.7/site-packages/git_pw/utils.py", line 138, in echo_via_pager
    output = _tabulate(output, headers, fmt)
  File "/usr/lib/python2.7/site-packages/git_pw/utils.py", line 77, in _tabulate
    writer.writerow(_text(x if x is not None else '') for x in item)
_csv.Error: sequence expected

@stephenfin
Copy link
Member

Let's try again. Can you check the latest master?

@john-mcnamara-intel
Copy link
Author

Thanks. There is still an issue in Python 2.7:

$ git-pw patch list RFC --column ID --column Name --column Date --limit 4 -f csv
Traceback (most recent call last):
  File "/usr/bin/git-pw", line 10, in <module>
    sys.exit(cli())
  File "/usr/lib/python2.7/site-packages/click/core.py", line 716, in __call__
    return self.main(*args, **kwargs)
  File "/usr/lib/python2.7/site-packages/click/core.py", line 696, in main
    rv = self.invoke(ctx)
  File "/usr/lib/python2.7/site-packages/click/core.py", line 1060, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/usr/lib/python2.7/site-packages/click/core.py", line 1060, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/usr/lib/python2.7/site-packages/click/core.py", line 889, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/usr/lib/python2.7/site-packages/click/core.py", line 534, in invoke
    return callback(*args, **kwargs)
  File "/usr/lib/python2.7/site-packages/click/decorators.py", line 17, in new_func
    return f(get_current_context(), *args, **kwargs)
  File "/usr/lib/python2.7/site-packages/git_pw/api.py", line 303, in new_func
    return ctx.invoke(f, *args, **kwargs)
  File "/usr/lib/python2.7/site-packages/click/core.py", line 534, in invoke
    return callback(*args, **kwargs)
  File "/usr/lib/python2.7/site-packages/git_pw/patch.py", line 265, in list_cmd
    utils.echo_via_pager(output, headers, fmt=fmt)
  File "/usr/lib/python2.7/site-packages/git_pw/utils.py", line 130, in echo_via_pager
    output = _tabulate(output, headers, fmt)
  File "/usr/lib/python2.7/site-packages/git_pw/utils.py", line 66, in _tabulate
    writer.writerow([six.ensure_str(h) for h in headers])
AttributeError: 'module' object has no attribute 'ensure_str'

@stephenfin
Copy link
Member

That's because you've got an old version of six installed. You need at least 1.12. If you're using a virtualenv, can you recreate it? If you're not, can you install a newer version of six?

@john-mcnamara-intel
Copy link
Author

That's because you've got an old version of six installed. You need at least 1.12.

You are right. Sorry about that. I thought that would be picked up from requirements.txt.

If you're using a virtualenv, can you recreate it?

I'm not using virtualenv. I'm installing git-pw directly into the system:

$ sudo python setup.py install

If you're not, can you install a newer version of six?

I've updated any versions that needed:

$ pip2 freeze | egrep "click|requests|tabulate|arrow|six"
arrow==0.10.0
click==6.6
requests==2.19.1
six==1.12.0
tabulate==0.8.3

The six issue is fixed. However, there is still an issue:

$ git-pw patch list RFC --column ID --column Name --column Date --limit 4 -f csv
Traceback (most recent call last):
  File "/usr/bin/git-pw", line 10, in <module>
    sys.exit(cli())
  File "/usr/lib/python2.7/site-packages/click/core.py", line 716, in __call__
    return self.main(*args, **kwargs)
  File "/usr/lib/python2.7/site-packages/click/core.py", line 696, in main
    rv = self.invoke(ctx)
  File "/usr/lib/python2.7/site-packages/click/core.py", line 1060, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/usr/lib/python2.7/site-packages/click/core.py", line 1060, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/usr/lib/python2.7/site-packages/click/core.py", line 889, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/usr/lib/python2.7/site-packages/click/core.py", line 534, in invoke
    return callback(*args, **kwargs)
  File "/usr/lib/python2.7/site-packages/click/decorators.py", line 17, in new_func
    return f(get_current_context(), *args, **kwargs)
  File "/usr/lib/python2.7/site-packages/git_pw/api.py", line 303, in new_func
    return ctx.invoke(f, *args, **kwargs)
  File "/usr/lib/python2.7/site-packages/click/core.py", line 534, in invoke
    return callback(*args, **kwargs)
  File "/usr/lib/python2.7/site-packages/git_pw/patch.py", line 265, in list_cmd
    utils.echo_via_pager(output, headers, fmt=fmt)
  File "/usr/lib/python2.7/site-packages/git_pw/utils.py", line 130, in echo_via_pager
    output = _tabulate(output, headers, fmt)
  File "/usr/lib/python2.7/site-packages/git_pw/utils.py", line 69, in _tabulate
    six.ensure_str(x if x is not None else '') for x in item])
  File "/usr/lib/python2.7/site-packages/six.py", line 884, in ensure_str
    raise TypeError("not expecting type '%s'" % type(s))
TypeError: not expecting type '<type 'int'>'

@stephenfin
Copy link
Member

Third times the charm? 🙈 New version pushed that handles ints too.

@john-mcnamara-intel
Copy link
Author

Sucess!

$ git-pw patch list RFC --column ID --column Name --column Date --limit 4 -f csv
"ID","Name","Date"
"59272","2 days ago","[RFC,v3] net/i40e: enable multi-queue Rx interrupt for VF"
"59107","7 days ago","[RFC] kni: properly translate pa2va for cloned mbuf"
"59106","7 days ago","[RFC,v3,4/4] app/testpmd: show the Rx/Tx burst mode description"
"59105","7 days ago","[RFC,v3,3/4] net/ice: support to get the Rx/Tx burst mode"

Thanks for the fix and your patience.

However, I'm now seeing a Unicode issue in another case.

Here is a dump from the DPDK patchwork which works:

$ git-pw patch list "bulk" --column ID --column Submitter
+-------+------------------------------------------+
|    ID | Submitter                                |
|-------+------------------------------------------|
| 59113 | Morten Brørup (mb@smartsharesystems.com) |
+-------+------------------------------------------+

However, with the CSV output it throws an exception

$ git-pw patch list "bulk" --column ID --column Submitter -f csv
Traceback (most recent call last):
  File "/usr/bin/git-pw", line 10, in <module>
    sys.exit(cli())
  File "/usr/lib/python2.7/site-packages/click/core.py", line 716, in __call__
    return self.main(*args, **kwargs)
  File "/usr/lib/python2.7/site-packages/click/core.py", line 696, in main
    rv = self.invoke(ctx)
  File "/usr/lib/python2.7/site-packages/click/core.py", line 1060, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/usr/lib/python2.7/site-packages/click/core.py", line 1060, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/usr/lib/python2.7/site-packages/click/core.py", line 889, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/usr/lib/python2.7/site-packages/click/core.py", line 534, in invoke
    return callback(*args, **kwargs)
  File "/usr/lib/python2.7/site-packages/click/decorators.py", line 17, in new_func
    return f(get_current_context(), *args, **kwargs)
  File "/usr/lib/python2.7/site-packages/git_pw/api.py", line 303, in new_func
    return ctx.invoke(f, *args, **kwargs)
  File "/usr/lib/python2.7/site-packages/click/core.py", line 534, in invoke
    return callback(*args, **kwargs)
  File "/usr/lib/python2.7/site-packages/git_pw/patch.py", line 265, in list_cmd
    utils.echo_via_pager(output, headers, fmt=fmt)
  File "/usr/lib/python2.7/site-packages/git_pw/utils.py", line 155, in echo_via_pager
    _echo_via_pager('less', output)
  File "/usr/lib/python2.7/site-packages/git_pw/utils.py", line 113, in _echo_via_pager
    c.stdin.write(line.encode(encoding, 'replace'))
UnicodeDecodeError: 'ascii' codec can't decode byte 0xc3 in position 0: ordinal not in range(128)
"ID","Submitter"
"59113","Morten Br

@stephenfin
Copy link
Member

Tracked in #49. Let me know if the fix I've pushed for that does the trick. Unicode is hard 😰

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants