Skip to content
This repository has been archived by the owner on Dec 10, 2018. It is now read-only.

Parser doesn't support absolute path on windows #234

Open
tanwov opened this issue Sep 1, 2016 · 14 comments
Open

Parser doesn't support absolute path on windows #234

tanwov opened this issue Sep 1, 2016 · 14 comments

Comments

@tanwov
Copy link

tanwov commented Sep 1, 2016

Using urlparse to get the path's schema ,but on windows ,it doesn't work well.
When loads file using absolute path , it would raise

raise ThriftParserError('ThriftPy does not support generating module '
'with path in protocol '{}''.format(
url_scheme))

@tanwov
Copy link
Author

tanwov commented Sep 1, 2016

Could it be wrotten like

if url_scheme in ('http', 'https'):
    data = urlopen(path).read()
else:
    try:
        with open(path) as fh:
            data = fh.read()
    except Exception as e:
        raise ThriftParserError('ThriftPy does not support generating module '
                                'with path in protocol \'{}\''.format(
                                    url_scheme),e)

@tanwov tanwov changed the title Parser doesn't support path on windows Parser doesn't support absolute path on windows Sep 1, 2016
@lxyu
Copy link
Contributor

lxyu commented Sep 2, 2016

Can you be more specific about the parameter on how to reproduce and why this code fixed the bug?

@tanwov
Copy link
Author

tanwov commented Sep 2, 2016

test_thrift= thriftpy.load(r'C:\test.thrift',module_name='test_thrift')
urlparse with return

ParseResult(scheme='e', netloc='', path='\test.thrift', params='', query='', fragment='')

# parser.py  line 487
url_scheme = urlparse(path).scheme
if url_scheme == '':
    with open(path) as fh:
        data = fh.read()
elif url_scheme in ('http', 'https'):
    data = urlopen(path).read()
else:
    raise ThriftParserError('ThriftPy does not support generating module '
                            'with path in protocol \'{}\''.format(
                                url_scheme))

because the url_scheme is e, it goes into the else branch and raise ThriftParserError

@lxyu
Copy link
Contributor

lxyu commented Sep 2, 2016

I see, I'll look into this problem later.

For now, you may want to check parser_fp function, located at https://github.com/eleme/thriftpy/blob/develop/thriftpy/parser/parser.py#L518

@euandekock
Copy link

I am also encountering this issue. My temporary fix was to edit thriftpy/parser/parser.py, and change line 488 from
if url_scheme == '':
to
if url_scheme in ('c', ''):

This will fix it for files located in the C: drive as this resolves to a uri protocol of c, we probably need to also add 'd', 'e', 'f' etc to be safe - or do something a bit more sensible.

@ha62791
Copy link

ha62791 commented Jan 27, 2017

The problem only occurs when I use pip to install thriftpy.
No such problem when I use conda to install.

Tested on:
Windows 7
Anaconda Python 3.5
thriftpy 0.3.8 (installed via conda)

@stonebig
Copy link

stonebig commented Apr 17, 2017

I confirm the suggested patch "#234 (comment)" is mandatory to have a working Thriftpy on WinPython.

I suspect the bug was not in 0.3.8, only in 0.3.9

@aawilson
Copy link

Theoretically, you try to open the path as a file as the fallback to unsupported schemes, rather than simply giving up. Let the filesystem figure out what it can and can't open rather than trying to preempt it.

@aawilson
Copy link

Alternatively, maybe it's a bad idea for "load" to magically coerce different path types to begin with, and the function should have separate treatments for "filesystem paths" and "URLs".

@markmnl
Copy link

markmnl commented Nov 13, 2017

What is the plan to fix this issue? Why wasn't the PR accepted?

As it stands Windows users cannot use packages that use thirftpy, e.g. happybase, because they import thriftpy and it falls over..

@jbaayen
Copy link

jbaayen commented Feb 22, 2018

Any update on the status of this issue?

@rubenhak
Copy link

Guys, I'm also hitting this issue when trying to use py_zipkin on windows.

@aawilson
Copy link

aawilson commented Jun 21, 2018

@wooparadog stays pretty busy, from what it sounds like. I agree that #285 is a good enough approach, we should accept it.

@ethe
Copy link
Member

ethe commented Dec 9, 2018

Hi all, I accept markmnl@0801f46 in thriftpy2 to fix it, thanks!

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

No branches or pull requests

10 participants