Hi Aditya, Looks like there are changes in this patch that are related to notifications, these should go into a separate commit as they are not related to encoding. Also the tests are failing in our pipeline: https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-tests/builds/110
Thanks Victoria && Joao On Mon, Jun 4, 2018 at 5:55 AM Aditya Toshniwal < aditya.toshni...@enterprisedb.com> wrote: > Hi Hackers, > > Please ignore previous patch. Fixed some linter issues. PFA updated patch. > > Thanks and Regards, > Aditya Toshniwal > Software Engineer | EnterpriseDB Software Solutions | Pune > "Don't Complain about Heat, Plant a tree" > > On Mon, Jun 4, 2018 at 10:53 AM, Aditya Toshniwal < > aditya.toshni...@enterprisedb.com> wrote: > >> Hi Hackers, >> >> PFA the updated patch on this. I have tried to add test cases to check >> for different encoding database. In the test run, it will create a >> database, fire a query for a string, check if we get the output and drops >> the database. >> Kindly review. >> >> Thanks and Regards, >> Aditya Toshniwal >> Software Engineer | EnterpriseDB Software Solutions | Pune >> "Don't Complain about Heat, Plant a tree" >> >> On Thu, May 31, 2018 at 6:42 PM, Dave Page <dp...@pgadmin.org> wrote: >> >>> Hi >>> >>> On Thu, May 31, 2018 at 1:20 AM, Aditya Toshniwal < >>> aditya.toshni...@enterprisedb.com> wrote: >>> >>>> Hi Victoria/Joao, >>>> >>>> On Thu, May 31, 2018 at 2:06 AM, Joao De Almeida Pereira < >>>> jdealmeidapere...@pivotal.io> wrote: >>>> >>>>> Hello Aditya, >>>>> >>>>> It looks ok and it passes CI. >>>>> >>>>> We have some recommendations: >>>>> - These look like 2 different changes so they should be in separated >>>>> commits >>>>> >>>> >>>> If you are talking of set client_encoding, then its not a bug. Its a >>>> choice given to Postgres DB user to change the encoding of the characters. >>>> Postgres will translate characters from Server Encoding to Client Encoding, >>>> and will throw error like mentioned previously. This link will help better >>>> - https://www.postgresql.org/docs/10/static/multibyte.html >>>> The actual bug was, even after changing the client encoding to >>>> SQL_ASCII, pgAdmin4 was not able to show the output as it was failing in >>>> encoding by psycopg2. The patch is for resolving that. >>>> >>>> >>>>> - Do we have test coverage for the bug that you are talking about? If >>>>> not we should, to ensure this problem does not happen again in a future >>>>> change. >>>>> >>>> >>>> It is not possible adding test cases for encoding related stuff, as >>>> Postgres support a lot many different types of encoding and conversions >>>> (refer above link) >>>> >>> >>> I was going to ask the same thing. Per >>> https://www.postgresql.org/docs/10/static/multibyte.html#id-1.6.10.5.7, >>> every characterset except SQL_ASCII can be converted to UTF-8, so we only >>> need to test that UTF-8 and some other charactersets besides SQL_ASCII >>> work, and then separately that SQL_ASCII with characters known not to be in >>> UTF-8 work. >>> >>> >>>> >>>>> Thanks >>>>> Victoria && Joao >>>>> >>>>> On Wed, May 30, 2018 at 3:06 AM Aditya Toshniwal < >>>>> aditya.toshni...@enterprisedb.com> wrote: >>>>> >>>>>> Hi Hackers, >>>>>> >>>>>> PFA updated patch after all the permutations, combinations for >>>>>> encoding for SQL_ASCII database. Also fixed a small glitch for sql >>>>>> editor connection status check. >>>>>> >>>>>> Please note, ERROR: invalid byte sequence for encoding "UTF8": 0xe9 >>>>>> 0x73 is a Postgres DB error and not pgAdmin4 error. >>>>>> >>>>>> <Image Deleted> >>>>>> >>>>>> You need to change client_encoding to the appropriate. After changing >>>>>> client_encoding using command - set client_encoding='XYZ', it will give >>>>>> not >>>>>> give error. >>>>>> >>>>>> <Image Deleted> >>>>>> >>>>>> Thanks and Regards, >>>>>> Aditya Toshniwal >>>>>> Software Engineer | EnterpriseDB Software Solutions | Pune >>>>>> "Don't Complain about Heat, Plant a tree" >>>>>> >>>>>> On Wed, May 23, 2018 at 10:13 AM, Aditya Toshniwal < >>>>>> aditya.toshni...@enterprisedb.com> wrote: >>>>>> >>>>>>> Thank you Victoria, Anthony. >>>>>>> >>>>>>> Thanks and Regards, >>>>>>> Aditya Toshniwal >>>>>>> Software Engineer | EnterpriseDB Software Solutions | Pune >>>>>>> "Don't Complain about Heat, Plant a tree" >>>>>>> >>>>>>> On Tue, May 22, 2018 at 7:15 PM, Victoria Henry <vhe...@pivotal.io> >>>>>>> wrote: >>>>>>> >>>>>>>> Hi Aditya, >>>>>>>> >>>>>>>> We made a minor change to make the patch so the python linter can >>>>>>>> pass. Attached is the change we made. >>>>>>>> Everything else looks good. >>>>>>>> >>>>>>>> Sincerely, >>>>>>>> >>>>>>>> Victoria & Anthony >>>>>>>> >>>>>>>> On Tue, May 22, 2018 at 4:46 AM Aditya Toshniwal < >>>>>>>> aditya.toshni...@enterprisedb.com> wrote: >>>>>>>> >>>>>>>>> Hi, >>>>>>>>> >>>>>>>>> PFA updated patch. Linter issues are fixed ( we dont have any >>>>>>>>> linter setup for python :-( ) >>>>>>>>> Regarding test cases, they run successfully on my system and the >>>>>>>>> reason it failed for pivotal is timeout exception. I am sorry I can't >>>>>>>>> help >>>>>>>>> with that. >>>>>>>>> >>>>>>>>> Traceback (most recent call last): >>>>>>>>> File >>>>>>>>> "/tmp/build/a453582b/pgadmin-repo/web/pgadmin/feature_tests/keyboard_shortcut_test.py", >>>>>>>>> line 52, in runTest >>>>>>>>> self._check_shortcuts() >>>>>>>>> File >>>>>>>>> "/tmp/build/a453582b/pgadmin-repo/web/pgadmin/feature_tests/keyboard_shortcut_test.py", >>>>>>>>> line 77, in _check_shortcuts >>>>>>>>> ") and contains(@class, 'open')]") >>>>>>>>> File >>>>>>>>> "/root/.pyenv/versions/pgadmin36/lib/python3.6/site-packages/selenium/webdriver/support/wait.py", >>>>>>>>> line 80, in until >>>>>>>>> raise TimeoutException(message, screen, stacktrace) >>>>>>>>> selenium.common.exceptions.TimeoutException: Message: >>>>>>>>> >>>>>>>>> Thanks and Regards, >>>>>>>>> Aditya Toshniwal >>>>>>>>> Software Engineer | EnterpriseDB Software Solutions | Pune >>>>>>>>> "Don't Complain about Heat, Plant a tree" >>>>>>>>> >>>>>>>>> On Tue, May 22, 2018 at 1:37 PM, Dave Page <dp...@pgadmin.org> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>>> Hi >>>>>>>>>> >>>>>>>>>> Pivotal's buildbot is showing problems with this patch: >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/66 >>>>>>>>>> (linter failed) >>>>>>>>>> >>>>>>>>>> https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-tests/builds/84 >>>>>>>>>> (tests failed) >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On Tue, May 22, 2018 at 7:05 AM, Aditya Toshniwal < >>>>>>>>>> aditya.toshni...@enterprisedb.com> wrote: >>>>>>>>>> >>>>>>>>>>> Hi Hackers, >>>>>>>>>>> >>>>>>>>>>> PFA patch for RM#3289 where decode error was thrown on querying >>>>>>>>>>> a SQL_ASCII database table. Please note, this problem occurs only on >>>>>>>>>>> windows. >>>>>>>>>>> Sample insert - insert into test_tab values ('é'); >>>>>>>>>>> >>>>>>>>>>> psycopg2 has a encodings dictionary where Postgres Database >>>>>>>>>>> Encodings are mapped to python equivalent. It uses 'ascii' decoder >>>>>>>>>>> of >>>>>>>>>>> python to decode for SQL_ASCII encoding. If data has characters >>>>>>>>>>> beyond the >>>>>>>>>>> limit of ascii then it failed. The solution would be to use utf_8 >>>>>>>>>>> decoder >>>>>>>>>>> instead of ascii. I tried setting the client_encoding using >>>>>>>>>>> set_client_encoding('UTF8') method of a psycopg2 connection but no >>>>>>>>>>> luck >>>>>>>>>>> (also its not allowed for async connection). I also tried executing >>>>>>>>>>> "SET >>>>>>>>>>> CLIENT_ENCODING='UTF8'" but it didn't work too. >>>>>>>>>>> So, as in the patch, I had to set encodings dict value directly >>>>>>>>>>> to 'utf_8' and it seems to be working. Please note, the same is >>>>>>>>>>> added to >>>>>>>>>>> psycopg3 milestones >>>>>>>>>>> https://github.com/psycopg/psycopg2/milestone/4 >>>>>>>>>>> >>>>>>>>>>> Also fixed a small glitch for sql editor connection status check. >>>>>>>>>>> >>>>>>>>>>> Kindly review. >>>>>>>>>>> >>>>>>>>>>> Thanks and Regards, >>>>>>>>>>> Aditya Toshniwal >>>>>>>>>>> Software Engineer | EnterpriseDB Software Solutions | Pune >>>>>>>>>>> "Don't Complain about Heat, Plant a tree" >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> -- >>>>>>>>>> Dave Page >>>>>>>>>> Blog: http://pgsnake.blogspot.com >>>>>>>>>> Twitter: @pgsnake >>>>>>>>>> >>>>>>>>>> EnterpriseDB UK: http://www.enterprisedb.com >>>>>>>>>> The Enterprise PostgreSQL Company >>>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>> >>>>>> >>>> >>> >>> >>> -- >>> Dave Page >>> Blog: http://pgsnake.blogspot.com >>> Twitter: @pgsnake >>> >>> EnterpriseDB UK: http://www.enterprisedb.com >>> The Enterprise PostgreSQL Company >>> >> >> >
