1) Fixed an issue where the process watcher dialog throws an error for

the database server which is already removed. Fixes #5985
2) Fixed cognitive complexity reported by SonarQube.
pull/37/head
Rahul Shirsat 2020-11-12 17:47:21 +05:30 committed by Akshay Joshi
parent a026f339c3
commit be386e77f2
8 changed files with 99 additions and 21 deletions

View File

@ -11,6 +11,7 @@ notes for it.
.. toctree::
:maxdepth: 1
release_notes_4_29
release_notes_4_28
release_notes_4_27
release_notes_4_26

View File

@ -0,0 +1,20 @@
************
Version 4.29
************
Release date: 2020-12-10
This release contains a number of bug fixes and new features since the release of pgAdmin4 4.28.
New features
************
Housekeeping
************
Bug fixes
*********
| `Issue #5985 <https://redmine.postgresql.org/issues/5985>`_ - Fixed an issue where the process watcher dialog throws an error for the database server which is already removed.

View File

@ -262,3 +262,21 @@ def remove_saved_passwords(user_id):
except Exception as _:
db.session.rollback()
raise
def does_server_exists(sid, user_id):
"""
This function will return True if server is existing for a user
:param sid: server id
:param user_id: user id
:return: Boolean
"""
# **kwargs parameter can be added to function to filter with more
# parameters.
try:
return True if Server.query.filter_by(
id=sid, user_id=user_id
).first() is not None else False
except Exception:
return False

View File

@ -23,6 +23,7 @@ import logging
from pgadmin.utils import u_encode, file_quote, fs_encoding, \
get_complete_file_path, get_storage_directory, IS_WIN
from pgadmin.browser.server_groups.servers.utils import does_server_exists
import pytz
from dateutil import parser
@ -68,9 +69,9 @@ class IProcessDesc(object, metaclass=ABCMeta):
try:
# check if file name is encoded with UTF-8
file = self.bfile.decode('utf-8')
except Exception as e:
str(e)
except Exception:
# do nothing if bfile is not encoded.
pass
path = get_complete_file_path(file)
path = file if path is None else path
@ -87,14 +88,7 @@ class IProcessDesc(object, metaclass=ABCMeta):
else:
last_dir = file
if IS_WIN:
if '\\' in last_dir:
if len(last_dir) == 1:
last_dir = last_dir.replace('\\', '\\\\')
else:
last_dir = last_dir.replace('\\', '/')
else:
last_dir = last_dir.replace('\\', '/')
last_dir = replace_path_for_win(last_dir)
return None if hasattr(self, 'is_import') and self.is_import \
else last_dir
@ -102,6 +96,16 @@ class IProcessDesc(object, metaclass=ABCMeta):
return None
def replace_path_for_win(last_dir=None):
if IS_WIN:
if '\\' in last_dir and len(last_dir) == 1:
last_dir = last_dir.replace('\\', '\\\\')
else:
last_dir = last_dir.replace('\\', '/')
return last_dir
class BatchProcess(object):
def __init__(self, **kwargs):
@ -625,6 +629,9 @@ class BatchProcess(object):
):
continue
if BatchProcess._operate_orphan_process(p):
continue
execution_time = None
stime = parser.parse(p.start_time)
@ -654,6 +661,29 @@ class BatchProcess(object):
return res
@staticmethod
def _operate_orphan_process(p):
if p and p.desc:
desc = loads(p.desc)
if does_server_exists(desc.sid, current_user.id) is False:
current_app.logger.warning(
_("Server with id '{0}' is either removed or does "
"not exists for the background process "
"'{1}'").format(desc.sid, p.pid)
)
try:
BatchProcess.acknowledge(p.pid)
except LookupError as lerr:
current_app.logger.warning(
_("Status for the background process '{0}' could "
"not be loaded.").format(p.pid)
)
current_app.logger.exception(lerr)
return True
return False
@staticmethod
def total_seconds(dt):
return round(dt.total_seconds(), 2)

View File

@ -141,11 +141,6 @@ class BackupMessage(IProcessDesc):
# It should never reach here.
return _("Unknown Backup")
# @property
# def current_storage_dir(self):
# return self.bfile if os.path.isdir(self.bfile) \
# else os.path.dirname(self.bfile)
@property
def message(self):
name, host, port = self.get_server_details()

View File

@ -199,11 +199,18 @@ class BatchProcessTest(BaseTestGenerator):
self.assertTrue(popen_mock.called)
@patch('os.path.realpath')
@patch('pgadmin.misc.bgprocess.processes.get_storage_directory')
@patch('pgadmin.misc.bgprocess.processes.get_complete_file_path')
@patch('pgadmin.misc.bgprocess.processes.Process')
@patch('pgadmin.misc.bgprocess.processes.BatchProcess.'
'update_process_info')
def _check_list(self, p, backup_obj, update_process_info_mock,
process_mock):
@patch('pgadmin.misc.bgprocess.processes.BatchProcess.'
'_operate_orphan_process')
def _check_list(self, p, backup_obj, _operate_orphan_process_mock,
update_process_info_mock, process_mock,
get_complete_file_path_mock, get_storage_directory_mock,
realpath_mock):
class TestMockProcess():
def __init__(self, desc, args, cmd):
self.pid = 1
@ -222,6 +229,10 @@ class BatchProcessTest(BaseTestGenerator):
self.class_params['cmd'])]
update_process_info_mock.return_value = [True, True]
get_complete_file_path_mock.return_value = self.class_params['bfile']
realpath_mock.return_value = self.class_params['bfile']
get_storage_directory_mock.return_value = '//'
_operate_orphan_process_mock.return_value = False
ret_value = p.list()
self.assertEqual(1, len(ret_value))

View File

@ -212,9 +212,11 @@ class BatchProcessTest(BaseTestGenerator):
@patch('pgadmin.misc.bgprocess.processes.Process')
@patch('pgadmin.misc.bgprocess.processes.BatchProcess.'
'update_process_info')
def _check_list(self, p, import_export_obj, update_process_info_mock,
process_mock,
get_storage_directory_mock, get_complete_file_path_mock,
@patch('pgadmin.misc.bgprocess.processes.BatchProcess.'
'_operate_orphan_process')
def _check_list(self, p, import_export_obj, _operate_orphan_process_mock,
update_process_info_mock, process_mock,
get_complete_file_path_mock, get_storage_directory_mock,
realpath_mock):
class TestMockProcess():
def __init__(self, desc, args, cmd):
@ -237,6 +239,7 @@ class BatchProcessTest(BaseTestGenerator):
get_complete_file_path_mock.return_value = self.params['filename']
realpath_mock.return_value = self.params['filename']
get_storage_directory_mock.return_value = '//'
_operate_orphan_process_mock.return_value = False
ret_value = p.list()
self.assertEqual(1, len(ret_value))

View File

@ -65,7 +65,7 @@ define([
var params = {
supported_types: ['sql', 'csv', '*'],
dialog_type: 'storage_dialog',
dialog_title: 'Storage Manager...',
dialog_title: 'Storage Manager',
btn_primary: undefined,
};