Le 19/04/2011 12:47, Dave Page a écrit : > On Tue, Apr 19, 2011 at 10:48 AM, Guillaume Lelarge > <[email protected]> wrote: >> Hi, >> >> Le 19/04/2011 10:40, Dave Page a écrit : >>> [...] >>> I've tweaked the wording on the message in this patch as I wasn't >>> entirely happy with the phrasing, >> >> Good idea. >> >>> but whilst doing so I noticed there >>> doesn't seem to be any error checking to handle the case where the >>> extension cannot be installed (for example, because the user is using >>> a hosting site which hasn't installed the contrib modules). >>> >> >> We first check if the extension is available (query on >> pg_available_extensions). bUT I should probably add a check on the >> CREATE EXTENSION query because it may fail (if you're not superuser for >> example). >> >>> There's also no way to permanently suppress the message box, which >>> could become very annoying. >>> >> >> Yeah, I didn't think about it yesterday. That would be good to add. >> >>> Final thought - we also have a guru hint about the admin pack >>> (instrumentation.html). Shouldn't this patch touch the same places as >>> that hint (as well as anywhere else it makes sense)? >>> >> >> I've been thinking about it. I was wondering if we should have a >> specific hint for 8.1 till 9.0 and another one for 9.1. >> >> And one other issue: we may be connected to another database than the >> postgres one. I should probably check that too. > > I was thinking more that we shouldn't have a hint in one place, and > offer to fix it in another. We should have the hint, and offer to fix > the problem in all places (frmHint can handle "fix" options iirc - for > example, it can vacuum for you). >
Got it :) This patch should fix all the remaining issues, shouldn't it? -- Guillaume http://www.postgresql.fr http://dalibo.com
>From a12dafb2395544e01bc0329087ce9d95d1bf5474 Mon Sep 17 00:00:00 2001 From: Guillaume Lelarge <[email protected]> Date: Tue, 19 Apr 2011 22:56:11 +0200 Subject: [PATCH] Use frmHint to fix the addition of adminpack --- docs/en_US/hints/instrumentation91_with.html | 28 ++++++++++++ docs/en_US/hints/instrumentation91_without.html | 28 ++++++++++++ pgadmin/frm/frmHint.cpp | 14 ++++++ pgadmin/frm/frmStatus.cpp | 55 +++++++++++------------ pgadmin/include/frm/frmHint.h | 32 +++++++------ pgadmin/schema/pgDatabase.cpp | 23 +++++++++- 6 files changed, 135 insertions(+), 45 deletions(-) create mode 100644 docs/en_US/hints/instrumentation91_with.html create mode 100644 docs/en_US/hints/instrumentation91_without.html diff --git a/docs/en_US/hints/instrumentation91_with.html b/docs/en_US/hints/instrumentation91_with.html new file mode 100644 index 0000000..69a9ecc --- /dev/null +++ b/docs/en_US/hints/instrumentation91_with.html @@ -0,0 +1,28 @@ +<html> + +<head> +<meta http-equiv="Content-Type" content="text/html; charset=windows-1252"> +<link rel="STYLESHEET" type="text/css" href="../pgadmin3.css"> +<title>Guru Hints</title> +</head> + +<body> + +<h3>Server instrumentation</h3> +<p> +The server lacks instrumentation functions. +</p> +<p> +pgAdmin III uses some support functions that are not available by default +in all PostgreSQL versions. These enable some tasks that make life easier when +dealing with log files and configuration files. +</p> +<p>The adminpack is installed and activated by default if you are running the +one-click installer of PostgreSQL. On Unix, you may have to install the contrib +package, either with the your package installer tool or by compilation.</p> + +<p>One your extension is installed, you only need to click on the "Fix" +button, so that pgAdmin installs this extension in your maintenance database. +</p> +</body> +</html> diff --git a/docs/en_US/hints/instrumentation91_without.html b/docs/en_US/hints/instrumentation91_without.html new file mode 100644 index 0000000..69a9ecc --- /dev/null +++ b/docs/en_US/hints/instrumentation91_without.html @@ -0,0 +1,28 @@ +<html> + +<head> +<meta http-equiv="Content-Type" content="text/html; charset=windows-1252"> +<link rel="STYLESHEET" type="text/css" href="../pgadmin3.css"> +<title>Guru Hints</title> +</head> + +<body> + +<h3>Server instrumentation</h3> +<p> +The server lacks instrumentation functions. +</p> +<p> +pgAdmin III uses some support functions that are not available by default +in all PostgreSQL versions. These enable some tasks that make life easier when +dealing with log files and configuration files. +</p> +<p>The adminpack is installed and activated by default if you are running the +one-click installer of PostgreSQL. On Unix, you may have to install the contrib +package, either with the your package installer tool or by compilation.</p> + +<p>One your extension is installed, you only need to click on the "Fix" +button, so that pgAdmin installs this extension in your maintenance database. +</p> +</body> +</html> diff --git a/pgadmin/frm/frmHint.cpp b/pgadmin/frm/frmHint.cpp index 7d46251..ed408e8 100644 --- a/pgadmin/frm/frmHint.cpp +++ b/pgadmin/frm/frmHint.cpp @@ -114,6 +114,20 @@ hintArray[] = HINT_CANSUPPRESS }, { + HINT_INSTRUMENTATION_91_WITH, + __("Server instrumentation not installed"), + 0, + wxT("instrumentation91_with"), + HINT_CANSUPPRESS | HINT_CANFIX + }, + { + HINT_INSTRUMENTATION_91_WITHOUT, + __("Server instrumentation not installed"), + 0, + wxT("instrumentation91_without"), + HINT_CANSUPPRESS + }, + { HINT_ENCODING_UNICODE, __("Database encoding is Unicode"), 0, diff --git a/pgadmin/frm/frmStatus.cpp b/pgadmin/frm/frmStatus.cpp index 0231a6b..ed11ccb 100644 --- a/pgadmin/frm/frmStatus.cpp +++ b/pgadmin/frm/frmStatus.cpp @@ -785,7 +785,8 @@ void frmStatus::AddXactPane() void frmStatus::AddLogPane() { - bool forceCheck = false; + int rc = -1; + wxString hint = HINT_INSTRUMENTATION; // Create panel wxPanel *pnlLog = new wxPanel(this); @@ -823,39 +824,37 @@ void frmStatus::AddLogPane() logList = (ctlListView *)lstLog; // We don't need this report (but we need the pane) - // if the server release is 9.1 or more and the server has no adminpack - if (connection->BackendMinimumVersion(9, 1) && - !connection->HasFeature(FEATURE_FILEREAD)) + // if server release is less than 8.0 or if server has no adminpack + if (!(connection->BackendMinimumVersion(8, 0) && + connection->HasFeature(FEATURE_FILEREAD))) { - // Search the adminpack extension - pgSet *set = connection->ExecuteSet(wxT("SELECT 1 FROM pg_available_extensions WHERE name='adminpack'")); - if (set->NumRows() == 1) + // if the server release is 9.1 or more and the server has no adminpack + if (connection->BackendMinimumVersion(9, 1)) { - int answer = wxMessageBox(_("The adminpack extension is required to view log files, but is not currently installed. Would you like to install it, if available?"), - _("Install adminpack extension?"), - wxYES_NO, this); - if (answer == wxYES) - { - connection->ExecuteScalar(wxT("CREATE EXTENSION adminpack")); - forceCheck = true; - } + // Search the adminpack extension + pgSet *set = connection->ExecuteSet(wxT("SELECT 1 FROM pg_available_extensions WHERE name='adminpack'")); + if (set->NumRows() == 1) + hint = HINT_INSTRUMENTATION_91_WITH; + else + hint = HINT_INSTRUMENTATION_91_WITHOUT; + delete set; } - delete set; - } - // if server release is less than 8.0 or if server has no adminpack - if (!(connection->BackendMinimumVersion(8, 0) && - connection->HasFeature(FEATURE_FILEREAD, forceCheck))) - { if (connection->BackendMinimumVersion(8, 0)) - frmHint::ShowHint(this, HINT_INSTRUMENTATION); + rc = frmHint::ShowHint(this, hint); - logList->InsertColumn(logList->GetColumnCount(), _("Message"), wxLIST_FORMAT_LEFT, 800); - logList->InsertItem(logList->GetItemCount(), _("Logs are not available for this server."), -1); - logList->Enable(false); - logTimer = NULL; - // We're done - return; + if (rc == HINT_RC_FIX) + connection->ExecuteVoid(wxT("CREATE EXTENSION adminpack"), true); + + if (!connection->HasFeature(FEATURE_FILEREAD, true)) + { + logList->InsertColumn(logList->GetColumnCount(), _("Message"), wxLIST_FORMAT_LEFT, 800); + logList->InsertItem(logList->GetItemCount(), _("Logs are not available for this server."), -1); + logList->Enable(false); + logTimer = NULL; + // We're done + return; + } } // Add each column to the list control diff --git a/pgadmin/include/frm/frmHint.h b/pgadmin/include/frm/frmHint.h index 5a707db..d7cfcff 100644 --- a/pgadmin/include/frm/frmHint.h +++ b/pgadmin/include/frm/frmHint.h @@ -13,21 +13,23 @@ #define __FRMHINT -#define HINT_CONNECTSERVER wxT("conn-listen") -#define HINT_MISSINGHBA wxT("conn-hba") -#define HINT_MISSINGIDENT wxT("conn-ident") -#define HINT_PRIMARYKEY wxT("pk") -#define HINT_FKINDEX wxT("fki") -#define HINT_VACUUM wxT("vacuum") -#define HINT_VACUUM_FULL wxT("vacuum-full") -#define HINT_QUERYRUNTIME wxT("query-runtime") -#define HINT_INSTRUMENTATION wxT("instrumentation") -#define HINT_ENCODING_ASCII wxT("encoding-ascii") -#define HINT_ENCODING_UNICODE wxT("encoding-unicode") -#define HINT_READONLY_NOPK wxT("view-without-pk") -#define HINT_AUTOVACUUM wxT("autovacuum") -#define HINT_OBJECT_EDITING wxT("object-editing") -#define HINT_SAVING_PASSWORDS wxT("saving-passwords") +#define HINT_CONNECTSERVER wxT("conn-listen") +#define HINT_MISSINGHBA wxT("conn-hba") +#define HINT_MISSINGIDENT wxT("conn-ident") +#define HINT_PRIMARYKEY wxT("pk") +#define HINT_FKINDEX wxT("fki") +#define HINT_VACUUM wxT("vacuum") +#define HINT_VACUUM_FULL wxT("vacuum-full") +#define HINT_QUERYRUNTIME wxT("query-runtime") +#define HINT_INSTRUMENTATION wxT("instrumentation") +#define HINT_INSTRUMENTATION_91_WITH wxT("instrumentation91_with") +#define HINT_INSTRUMENTATION_91_WITHOUT wxT("instrumentation91_without") +#define HINT_ENCODING_ASCII wxT("encoding-ascii") +#define HINT_ENCODING_UNICODE wxT("encoding-unicode") +#define HINT_READONLY_NOPK wxT("view-without-pk") +#define HINT_AUTOVACUUM wxT("autovacuum") +#define HINT_OBJECT_EDITING wxT("object-editing") +#define HINT_SAVING_PASSWORDS wxT("saving-passwords") #define HINT_RC_FIX 42 diff --git a/pgadmin/schema/pgDatabase.cpp b/pgadmin/schema/pgDatabase.cpp index 9f9c79a..634640c 100644 --- a/pgadmin/schema/pgDatabase.cpp +++ b/pgadmin/schema/pgDatabase.cpp @@ -244,6 +244,7 @@ bool pgDatabase::GetCanHint() void pgDatabase::ShowHint(frmMain *form, bool force) { wxArrayString hints; + int rc = -1; if (encoding == wxT("SQL_ASCII")) hints.Add(HINT_ENCODING_ASCII); @@ -257,10 +258,28 @@ void pgDatabase::ShowHint(frmMain *form, bool force) if (GetServer()->GetConnection() == GetConnection() && GetConnection()->BackendMinimumVersion(8, 0) && !GetConnection()->HasFeature(FEATURE_FILEREAD)) - hints.Add(HINT_INSTRUMENTATION); + { + // if the server release is 9.1 or more and the server has no adminpack + if (GetConnection()->BackendMinimumVersion(9, 1)) + { + // Search the adminpack extension + pgSet *set = GetConnection()->ExecuteSet(wxT("SELECT 1 FROM pg_available_extensions WHERE name='adminpack'")); + if (set->NumRows() == 1) + hints.Add(HINT_INSTRUMENTATION_91_WITH); + else + hints.Add(HINT_INSTRUMENTATION_91_WITHOUT); + delete set; + } + else + hints.Add(HINT_INSTRUMENTATION); + } if (force || !hintShown) - frmHint::ShowHint(form, hints, GetFullIdentifier(), force); + { + rc = frmHint::ShowHint(form, hints, GetFullIdentifier(), force); + if (rc == HINT_RC_FIX) + GetConnection()->ExecuteVoid(wxT("CREATE EXTENSION adminpack"), true); + } hintShown = true; } -- 1.7.1
-- Sent via pgadmin-hackers mailing list ([email protected]) To make changes to your subscription: http://www.postgresql.org/mailpref/pgadmin-hackers
