ODM2 / WOFpy

A server-side implementation of CUAHSI's Water One Flow service stack in Python.
http://odm2.github.io/WOFpy/
9 stars 9 forks source link

Unicode #188

Closed ocefpaf closed 7 years ago

ocefpaf commented 7 years ago

@lsetiawan this is not ready for review yet. I am not sure if it will solve our problems.

BTW, do we have the bug as a test case? B/c the tests were passing before, right?

lsetiawan commented 7 years ago

BTW, do we have the bug as a test case? B/c the tests were passing before, right?

I did make a test before #156 ... Not sure how robust it is though.

ocefpaf commented 7 years ago

I did make a test before #156 ... Not sure how robust it is though.

I take a weak test over nothing any day :smile: (But I don't think that is weak.)

We can try that and @sreeder's bug report.

lsetiawan commented 7 years ago

@ocefpaf OMG IT'S PASSING!! 😮 YOU ARE AWESOME!

ocefpaf commented 7 years ago

@ocefpaf OMG IT'S PASSING!! 😮 YOU ARE AWESOME!

You mean passing @sreeder case? I don't really think our tests here caught the issue in the first place :smile:

lsetiawan commented 7 years ago

You mean passing @sreeder case? I don't really think our tests here caught the issue in the first place

Idk. Lemme test locally.

ocefpaf commented 7 years ago

GitHub cannot even handle the diff :rofl:

screenshot from 2017-09-01 16-54-52

lsetiawan commented 7 years ago

@ocefpaf I tested similar request like @sreeder test case

.../postgresqlodm2timeseries/rest/1_1/GetValues?location=postgresqlodm2timeseries:desk_sensors&variable=postgresqlodm2timeseries:Decagon_CTD-10_Temp

From her description:

The character is the trademark symbol located in the organization definition field. It is connected to a postgres database.

I added: Decagon Devicesâ„¢ to Organization Description.

Now I get the error below, running the code with this PR's branch.

<ns0:Fault xmlns:ns0="http://schemas.xmlsoap.org/soap/envelope/">
<faultcode>soap11env:Server</faultcode>
<faultstring>'unicode' does not have the buffer interface</faultstring>
<faultactor/>
</ns0:Fault>
lsetiawan commented 7 years ago

Here's the full traceback:

ERROR:spyne.application.server:'unicode' does not have the buffer interface
Traceback (most recent call last):
  File "/home/ubuntu/miniconda/envs/wofpydev/lib/python2.7/site-packages/spyne/application.py", line 151, in process_request
    ctx.out_object = self.call_wrapper(ctx)
  File "/home/ubuntu/miniconda/envs/wofpydev/lib/python2.7/site-packages/spyne/application.py", line 235, in call_wrapper
    retval = ctx.descriptor.service_class.call_wrapper(ctx)
  File "/home/ubuntu/miniconda/envs/wofpydev/lib/python2.7/site-packages/spyne/service.py", line 209, in call_wrapper
    return ctx.function(ctx, *args)
  File "/home/ubuntu/wofpy-dev/wof/apps/spyned_1_1.py", line 208, in GetValues
    authToken
  File "/home/ubuntu/wofpy-dev/wof/apps/spyned_1_1.py", line 184, in GetValuesObject
    namespacedef_=NSDEF)
  File "/home/ubuntu/wofpy-dev/wof/WaterML_1_1.py", line 3472, in export
    outfile.write(u'<%s%s%s' % (namespace_, name_, namespacedef_ and ' ' + namespacedef_ or '', ))
TypeError: 'unicode' does not have the buffer interface
ocefpaf commented 7 years ago

Thanks! That is good. We need to add that as a test here in order to make progress.

lsetiawan commented 7 years ago

@ocefpaf The current test is actually capturing the encoding/decoding error. Your change https://github.com/ODM2/WOFpy/pull/188#discussion-diff-136649344R50 makes the test different from the actual application https://github.com/ODM2/WOFpy/blob/master/wof/apps/spyned_1_1.py#L38. This is why test is passing while application is failing. When I changed the test back to use io.BytesIO(). I get test failures of

...
>       outfile.write(u'<%s%s%s' % (namespace_, name_, namespacedef_ and ' ' + namespacedef_ or '', ))
E       TypeError: 'unicode' does not have the buffer interface

wof/WaterML_1_1.py:3472: TypeError
lsetiawan commented 7 years ago

When I changed the BytesIO() to StringIO() on the actual application, this is my traceback error:

...
  File "/home/ubuntu/wofpy-dev/wof/WaterML_1_1.py", line 408, in quote_xml_aux
    inStr = inStr.decode('utf-8')
  File "/home/ubuntu/miniconda/envs/wofpydev/lib/python2.7/encodings/utf_8.py", line 16, in decode
    return codecs.utf_8_decode(input, errors, True)
UnicodeEncodeError: 'ascii' codec can't encode character u'\u2122' in position 18: ordinal not in range(128)
lsetiawan commented 7 years ago

To follow up https://github.com/ODM2/WOFpy/pull/188#issuecomment-326686420.

I know I'm not supposed to do unicode whack-a-mole, but when I encode then decode inStr The problem goes away. inStr = inStr.encode('utf-8').decode('utf-8') This is also changing the BytesIO to StringIO.

lsetiawan commented 7 years ago

Looking into things deeper, it appears that the type of string for inStr is unicode.

ocefpaf commented 7 years ago

I know I'm not supposed to do unicode whack-a-mole, but when I encode then decode inStr The problem goes away. inStr = inStr.encode('utf-8').decode('utf-8') This is also changing the BytesIO to StringIO.

Changing only that is not a problem b/c inStr is the data entrypoint and BytesIO or StringIO is the endpoint. I want to avoid are internal encoding/decoding. However, to change that we need to be sure that is expected at those points.

lsetiawan commented 7 years ago

What's the status of this PR?

ocefpaf commented 7 years ago

What's the status of this PR?

Unresolved. Here is my take on this:

If your fix worked for @sreeder setup go with that and close this one until we can answer those questions and lay a strategy to move forward.

emiliom commented 7 years ago

this PR cannot address the issue until we "beef up" the tests to actually reproduce the errors; the green status here means nothing;

What's the specific github issue or bug originally addressed by this PR? I've lost track. (I realize there is a broader unicode handling problem in wofpy, but it'd still be helpful to know what specific error report generated this PR, despite the whack-a-mole situation)

If your fix worked for @sreeder setup go with that and close this one until we can answer those questions and lay a strategy to move forward.

FYI, Stephanie is starting maternity leave this week, probably today. I don't know if there's anyone in Jeff's team who will be able to continue to do wofpy implementation testing and engagement with us during Stephanie's absence.

lsetiawan commented 7 years ago

What's the specific github issue or bug originally addressed by this PR?

https://github.com/ODM2/WOFpy/issues/148#issuecomment-326420842

sreeder commented 7 years ago

@lsetiawan @emiliom I am still in today, I will be out tomorrow though. So if you guys would like me to test out this issue let me know!

lsetiawan commented 7 years ago

@sreeder Go ahead and try out this branch and see if it works for you. Thanks.

sreeder commented 7 years ago

@lsetiawan this is going to sound really novice, but how do I check out this branch since it is not on the ODM2 wofpy repo? :-)

ocefpaf commented 7 years ago

@sreeder this should work:

git clone https://github.com/ocefpaf/WOFpy.git
cd WOFpy
git checkout unicode
sreeder commented 7 years ago

@ocefpaf @lsetiawan @emiliom
I am still getting an error: but it is a different one.

screen shot 2017-09-18 at 2 31 27 pm
ocefpaf commented 7 years ago

I am still getting an error: but it is a different one.

Thanks for testing this @sreeder. Unless we can get that application as a test in this repo will be driving blind here. @lsetiawan can we have a call sometime to see if we can set that test up?

sreeder commented 7 years ago

@ocefpaf would it help if I gave you the exact text that is failing from my database?

"Since 1967, Stroud™ Water Research Center has been focused on one thing — fresh water."

ocefpaf commented 7 years ago

@ocefpaf would it help if I gave you the exact text that is failing from my database?

Thanks @sreeder. The main blocker, at least to me, is to set a database on Travis-CI for the testing. I am looking into it.

lsetiawan commented 7 years ago

@ocefpaf Can we use the testing method like ODM2PythonAPI? That repo spins up a database within Travis.

lsetiawan commented 7 years ago

Like I said on https://github.com/ODM2/WOFpy/pull/188#issuecomment-326689885. The current test mechanic is different than the actual application currently.

ocefpaf commented 7 years ago

@ocefpaf Can we use the testing method like ODM2PythonAPI? That repo spins up a database within Travis.

That is exactly what I was going to try :smile:

lsetiawan commented 7 years ago

I am going to merge this PR for now and further change things as I mention in https://github.com/ODM2/WOFpy/pull/188#issuecomment-326686420

miguelcleon commented 7 years ago

@lsetiawan should I try installing this update?

ocefpaf commented 7 years ago

@lsetiawan should I try installing this update?

@miguelcleon please let us know the results if you do. This part of the code needs more testing and supporting unicode for Python 2 is still a complicated task.

miguelcleon commented 7 years ago

Ok now I got a different error from: http://odm2admin.cuahsi.org/wofpy/odm2timeseries/rest/1_1/GetSiteInfo?site=odm2timeseries:Rio%20Icacos%20Trib-IO

<ns0:Fault xmlns:ns0="http://schemas.xmlsoap.org/soap/envelope/">
<faultcode>soap11env:Server</faultcode>
<faultstring>'unicode' does not have the buffer interface</faultstring>
<faultactor/>
</ns0:Fault>
lsetiawan commented 7 years ago

@miguelcleon Go ahead and try my latest change. Thanks!