public inbox for [email protected]
help / color / mirror / Atom feedFrom: Dave Page <[email protected]>
To: Akshay Joshi <[email protected]>
Cc: Joao Pedro De Almeida Pereira <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: [pgadmin][patch] [GreenPlum] When user press Explain Plan and Explain analyze plan an error is displayed
Date: Tue, 20 Mar 2018 09:36:19 +0000
Message-ID: <CA+OCxowWOpZ9r+S2SvNgqncN6aFGpc4Rom7-S74LVEmP92Tdag@mail.gmail.com> (raw)
In-Reply-To: <CANxoLDecQruspbc0dyw6ARNAM6DYJF5pC_0UJPqUzAM9spQHYg@mail.gmail.com>
References: <CAE+jjakOhMJBMqsUaQuL9w4an0V4Q4nvY51K3GqWzm8jbj9cog@mail.gmail.com>
<CA+OCxowBzhOaJf1_w1gPhOR4TH3bdtKBsdqETAqSwK0n2GWezg@mail.gmail.com>
<CA+OCxoyT9yQU-ayjRa94UMaekGP7QhB3CNqaJ_A-j8t+4Jb4FA@mail.gmail.com>
<CAE+jjam6bc4HeZgFbRayxXOJVft6J7s=Xb7M8i54mX7tNCp08Q@mail.gmail.com>
<CA+OCxozK4FeKyugzWNc2aRC1cMONiQMbFgus7Aj=7U28CaKV=Q@mail.gmail.com>
<CANxoLDecQruspbc0dyw6ARNAM6DYJF5pC_0UJPqUzAM9spQHYg@mail.gmail.com>
I'm a little concerned that noone mentioned this earlier; I'm supposed to
be building the release this afternoon, and I expect this change to at the
very least be complex to fully test and verify. What's the ETA on the
patch? What steps are being taken to ensure it's correct and doesn't cause
regressions?
On Tue, Mar 20, 2018 at 7:51 AM, Akshay Joshi <[email protected]
> wrote:
> Hi Joao
>
> It seems that this fix broke the functionality of RM #2815. It is
> mentioned in the RM what needs to be fixed now and I am currently working
> on it.
> While fixing the issue following problem that I found
>
> - In "start_running_query.py" file, we need to remove check "if conn.connected()"
> from "__execute_query" function as we required exception to be thrown while
> executing the query to identify the ConnectionLost.
> - In "execute_query.js" we have used *axios* to execute the query and
> in case of exception, object is different then normal javascript response
> object.
> - We call following functions when exception or error comes and send
> the "*<object>.response.data*" as parameter
> - wasConnectionLostToServer(): Check for the readyState parameter,
> which is not the part of "<object>.response.data".
> - extractErrorMessage(): Check for the "responseJSON" and "
> responseJSON.info", which is not the part of
> "<object>.response.data".
> - is_pga_login_required(): Check for the "responseJSON" and "
> responseJSON.info", which is not the part of
> "<object>.response.data".
> - is_new_transaction_required(): Check for the "responseJSON" and "
> responseJSON.info", which is not the part of
> "<object>.response.data".
>
> From the above list, some of the function calls are generic where they
> need "responseJSON" and "responseJSON.info", so we can't change that.
> Possible solution could be pass one extra flag as parameter to identify the
> object is a axios response or javascript response to above functions and
> change the logic accordingly.
>
> Please let me know your thoughts or any other suggestion.
>
>
> On Fri, Feb 9, 2018 at 8:17 PM, Dave Page <[email protected]> wrote:
>
>> Thanks, applied.
>>
>> On Fri, Feb 9, 2018 at 2:35 PM, Joao De Almeida Pereira <
>> [email protected]> wrote:
>>
>>> Hello,
>>> Attached you can find the fix for the current pronlem
>>>
>>>
>>> On Fri, Feb 9, 2018 at 7:29 AM Dave Page <[email protected]> wrote:
>>>
>>>> Hi Joao,
>>>>
>>>> It looks like Jenkins has taken umbrage to this change, at least with
>>>> Python 3.x. Can you take a look please?
>>>>
>>>> https://jenkins.pgadmin.org/
>>>>
>>>> Thanks.
>>>>
>>>> On Fri, Feb 9, 2018 at 11:54 AM, Dave Page <[email protected]> wrote:
>>>>
>>>>> Thanks, patches applied.
>>>>>
>>>>> On Fri, Feb 2, 2018 at 10:50 PM, Joao De Almeida Pereira <
>>>>> [email protected]> wrote:
>>>>>
>>>>>> Hi Hackers,
>>>>>> This is quite a big patch in order to solve the problem with the
>>>>>> Explain Plan.
>>>>>>
>>>>>> We sent 2 patches that have the following:
>>>>>> *- update-javascript-packages.diff *
>>>>>> Add package:
>>>>>> is-docker to select a specific setting when running the Chrome
>>>>>> tests in
>>>>>> Docker
>>>>>>
>>>>>> Upgrade the version of:
>>>>>> - babel-loader
>>>>>> - extract-text-webpack-plugin
>>>>>> - jasmine-core
>>>>>> - jasmine-enzyme
>>>>>> - moment
>>>>>> *- explain-plan-greenplum.diff*
>>>>>> Extract SQLEditor.execute and SQLEditor._poll into their own files
>>>>>> and add test around them
>>>>>> Extract SQLEditor backend functions that start executing query to
>>>>>> their own files and add tests around it
>>>>>> Move the Explain SQL from the front-end and now pass the Explain
>>>>>> plan parameters as a JSON object in the start query call.
>>>>>> Extract the compile_template_name into a function that can be used
>>>>>> by the different places that try to select the version of the template and
>>>>>> the server type
>>>>>>
>>>>>>
>>>>>> Thanks
>>>>>> Joao
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> 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
>>>>
>>>
>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>
>
>
> --
> *Akshay Joshi*
>
> *Sr. Software Architect *
>
>
>
> *Phone: +91 20-3058-9517 <+91%2020%203058%209517>Mobile: +91 976-788-8246
> <+91%2097678%2088246>*
>
--
Dave Page
VP, Chief Architect, Tools & Installers
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
view thread (19+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: [pgadmin][patch] [GreenPlum] When user press Explain Plan and Explain analyze plan an error is displayed
In-Reply-To: <CA+OCxowWOpZ9r+S2SvNgqncN6aFGpc4Rom7-S74LVEmP92Tdag@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox