Skip to content

RemoteOperations::exec_command explicitly transfers LANG, LANGUAGE and LC_* envvars to the server side #187

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

46 changes: 45 additions & 1 deletion testgres/operations/remote_ops.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,12 @@ def exec_command(self, cmd, wait_exit=False, verbose=False, expect_error=False,

assert type(cmd_s) == str # noqa: E721

ssh_cmd = ['ssh', self.ssh_dest] + self.ssh_args + [cmd_s]
cmd_items = __class__._make_exec_env_list()
cmd_items.append(cmd_s)

env_cmd_s = ';'.join(cmd_items)

ssh_cmd = ['ssh', self.ssh_dest] + self.ssh_args + [env_cmd_s]

process = subprocess.Popen(ssh_cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
assert not (process is None)
Expand Down Expand Up @@ -510,6 +515,45 @@ def db_connect(self, dbname, user, password=None, host="localhost", port=5432):
)
return conn

@staticmethod
def _make_exec_env_list() -> list[str]:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

тут нужно self, либо добавить @staticmethod

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self тут не нужен. Это статический метод.

Хорошо, я добавлю этот декоратор.

Я там в других местах тоже статические методы определял...

Тогда я пока только в новом коде этого PR его добавлю, а потом, отдельным коммитом, добавим его во все остальные случаи.

PS. Я этот питон на ходу изучаю, поэтому многие вещи использую неправильно ))))

result = list[str]()
for envvar in os.environ.items():
if not __class__._does_put_envvar_into_exec_cmd(envvar[0]):
continue
qvalue = __class__._quote_envvar(envvar[1])
assert type(qvalue) == str # noqa: E721
result.append(envvar[0] + "=" + qvalue)
continue

return result

sm_envs_for_exec_cmd = ["LANG", "LANGUAGE"]

@staticmethod
def _does_put_envvar_into_exec_cmd(name: str) -> bool:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

тут нужно self, либо добавить @staticmethod

assert type(name) == str # noqa: E721
name = name.upper()
if name.startswith("LC_"):
return True
if name in __class__.sm_envs_for_exec_cmd:
return True
return False

@staticmethod
def _quote_envvar(value: str) -> str:
assert type(value) == str # noqa: E721
result = "\""
for ch in value:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тут лучше append использовать https://www.geeksforgeeks.org/difference-between-and-append-in-python/

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Виктория, что-то я не догоняю. Куда тут append использовать?

result - это str. У него нет метода append, но есть поддержка оператора +=, который по смыслу тот же append. Этот += тут и используется.

Copy link
Contributor

@demonolock demonolock Feb 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

типо так


       result = ['"']
        for ch in value:
            if ch == '"':
                result.append('\\"')
            elif ch == '\\':
                result.append('\\\\')
            else:
                result.append(ch)
        result.append('"')
        return ''.join(result)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ага. Не. Не надо нам такой "оптимизации" )

Во первых у нас тут строка. И её элементы могут хранится непрерывно, ну или по крайней мере питон там может как-то оптимизировать это дело. А если будет массив строк - то однозначно каждый элемент массива будет хранится отдельно. Плюс место под сам массив.

А во вторых у нас тут нет оператора сложения. Используется оператор +=.

С append можно заморочится выше - там я три массива складываю через +.

Но честно - если бы эти бестолочи (я про авторов питона) позволяли писать append(list1).append(list2), я бы его заюзал. А так - в топку.


25 лет назад на .NET считалось дурным тоном складывать строки. Юзайте, говорят, StringBuilder. Но потом все дружно на него забили ))

if ch == "\"":
result += "\\\""
elif ch == "\\":
result += "\\\\"
else:
result += ch
result += "\""
return result


def normalize_error(error):
if isinstance(error, bytes):
Expand Down
79 changes: 79 additions & 0 deletions tests/test_simple_remote.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,79 @@ def test_custom_init(self):
# there should be no trust entries at all
self.assertFalse(any('trust' in s for s in lines))

def test_init__LANG_С(self):
# PBCKP-1744
prev_LANG = os.environ.get("LANG")

try:
os.environ["LANG"] = "C"

with get_remote_node(conn_params=conn_params) as node:
node.init().start()
finally:
__class__.helper__restore_envvar("LANG", prev_LANG)

def test_init__unk_LANG_and_LC_CTYPE(self):
# PBCKP-1744
prev_LANG = os.environ.get("LANG")
prev_LANGUAGE = os.environ.get("LANGUAGE")
prev_LC_CTYPE = os.environ.get("LC_CTYPE")
prev_LC_COLLATE = os.environ.get("LC_COLLATE")

try:
# TODO: Pass unkData through test parameter.
unkDatas = [
("UNKNOWN_LANG", "UNKNOWN_CTYPE"),
("\"UNKNOWN_LANG\"", "\"UNKNOWN_CTYPE\""),
("\\UNKNOWN_LANG\\", "\\UNKNOWN_CTYPE\\"),
("\"UNKNOWN_LANG", "UNKNOWN_CTYPE\""),
("\\UNKNOWN_LANG", "UNKNOWN_CTYPE\\"),
("\\", "\\"),
("\"", "\""),
]

for unkData in unkDatas:
logging.info("----------------------")
logging.info("Unk LANG is [{0}]".format(unkData[0]))
logging.info("Unk LC_CTYPE is [{0}]".format(unkData[1]))

os.environ["LANG"] = unkData[0]
os.environ.pop("LANGUAGE", None)
os.environ["LC_CTYPE"] = unkData[1]
os.environ.pop("LC_COLLATE", None)

assert os.environ.get("LANG") == unkData[0]
assert not ("LANGUAGE" in os.environ.keys())
assert os.environ.get("LC_CTYPE") == unkData[1]
assert not ("LC_COLLATE" in os.environ.keys())

while True:
try:
with get_remote_node(conn_params=conn_params):
pass
except testgres.exceptions.ExecUtilException as e:
#
# Example of an error message:
#
# warning: setlocale: LC_CTYPE: cannot change locale (UNKNOWN_CTYPE): No such file or directory
# postgres (PostgreSQL) 14.12
#
errMsg = str(e)

logging.info("Error message is: {0}".format(errMsg))

assert "LC_CTYPE" in errMsg
assert unkData[1] in errMsg
assert "warning: setlocale: LC_CTYPE: cannot change locale (" + unkData[1] + "): No such file or directory" in errMsg
assert "postgres" in errMsg
break
raise Exception("We expected an error!")
finally:
__class__.helper__restore_envvar("LANG", prev_LANG)
__class__.helper__restore_envvar("LANGUAGE", prev_LANGUAGE)
__class__.helper__restore_envvar("LC_CTYPE", prev_LC_CTYPE)
__class__.helper__restore_envvar("LC_COLLATE", prev_LC_COLLATE)

def test_double_init(self):
with get_remote_node(conn_params=conn_params).init() as node:
# can't initialize node more than once
Expand Down Expand Up @@ -994,6 +1067,12 @@ def test_child_process_dies(self):
# try to handle children list -- missing processes will have ptype "ProcessType.Unknown"
[ProcessProxy(p) for p in children]

def helper__restore_envvar(name, prev_value):
if prev_value is None:
os.environ.pop(name, None)
else:
os.environ[name] = prev_value


if __name__ == '__main__':
if os_ops.environ('ALT_CONFIG'):
Expand Down