Re: Updated patch: Add support for execution of jobs on a remote database

From: Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: Updated patch: Add support for execution of jobs on a remote database
Date: 2008-12-26 09:57:48
Message-ID: 4954AA9C.7020309@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi All,

Please find the updated patches for the same as per the instructions
from Dave Page.
>> - Add support for execution of jobs on a remote database in pgAgent.
>> (primarily for hot standby support). This essentially means allowing a
>> full connection string to be specified in place of the current database
>> name option.
>
> <snip comprehensive description>
>
> Hi Ashesh,
>
> I haven't tested fully at this stage, just eyeball reviewed the code,
> made sure it builds (on Windows) and checked out the UI. First
> impression is that it's well thought out and nicely implemented. I do
> have a few comments though - mostly minor:
>
> - The pgTable::HasColumn() static function seems like an unusual fit
> in that class. Yes, it's a table object, but that class is primarily
> concerned with representing a table in the treeview. It seems to me
> that a non-static pgConn::TableHasColumn() function might make more
> sense, and would probably look cleaner at the call sites.
Done.
> - The database icon in dlgSelectDatabase seems to be missing (I just
> get a blank space where it should be).
Done.
> - The height of the connection string textbox is inconsistent with all
> other textboxes. It should be sized as per the other one line text
> boxes, and the button altered to match it.
Done.
> - Any new headers should be added to include/precomp.h (we all forget
> to do that!)
Done.
> - In pgAgent, why is connInfo declared as a struct and not a class?
Done.
>
> - pgAdmin seems to be able to handle connecting to an old-style schema
> (which is good), but I didn't see any code in pgAgent to do similar. I
> would suggest we do something like the following, which should allow
> for future changes:
>
> * Add an SQL function pgagent_schema_version() which returns an int
> (with a value of 3 for this version - we'll bump the package to
> 3.0.0).
Introduced a new function pgagent_schema_version() in the pgagent.sql
Also, written into the pgagent_update.sql for unreadability.
> * Add a check to pgAgent in (MainLoop()) - upon setup of the primary
> connection, if pgagent_schema_version() does not exist, then exit with
> an error asking the user to upgrade the schema.
Done.
Checking for pgagent.pgagent_schema_version exists or not and also check if
return value of this function matches with the major version of pgagent.
> * If it does exist, check the value it returns against MAJOR_VERSION
> - a pre-processor macro we can set in cmake from
> CPACK_PACKAGE_VERSION_MAJOR - if it doesn't match, exit with an error
> telling the user there is a schema version mismatch.
Done.
Made changes in CMakeList.txt for adding definition of
PGAGENT_VERSION_MAJOR,
PGAGENT_VERSION_MINOR, PGAGENT_VERSION_PATCH in the Makefile.

BTW, changed the version of pgagent to 3.0.1 in the CMakeLists.txt

--
Regards,
Ashesh Vashi

EnterpriseDB INDIA: http://www.enterprisedb.com

Attachment Content-Type Size
pgadmin_agent_remotedb_v2.patch text/x-patch 37.7 KB
pgagent_remotedb_v2.patch text/x-patch 22.4 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Ashesh Vashi 2008-12-26 10:03:28 Re: Updated patch: Add support for execution of jobs on a remote database
Previous Message svn 2008-12-19 11:12:54 SVN Commit by dpage: r7524 - trunk/www/download