Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Fix set_auto_conf with single quotes#153

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
dmitry-lipetsk merged 5 commits intomasterfromfix-set-auto-conf
Dec 4, 2024

Conversation

demonolock
Copy link
Contributor

@demonolockdemonolock commentedDec 3, 2024
edited by dmitry-lipetsk
Loading

This patch improves an PostgresNode::set_auto_conf method

New implementation does the following things:

  • escaping '\n', '\r', '\t', '\b' and '\' symbols
  • translating bool-values into 'on|off'
  • saving a presentation of values those were not changed

This patch closes a problem with storing a command line with quotes in postgres' configuration files.

A test is provided.


It is a temporary solution to exist problems. In the future we should work with configuration files a more cleverly:

  • do not reorder declarations
  • do not remove comments

@dmitry-lipetsk
Copy link
Collaborator

dmitry-lipetsk commentedDec 3, 2024
edited
Loading

Postgres' functions for escaping and unescaping configuration property values.

/** Strip the quotes surrounding the given string, and collapse any embedded* '' sequences and backslash escapes.*/char*DeescapeQuotedString(constchar*s)

https://github.com/postgres/postgres/blob/e3fa2b037c6f0f435838e99200050dc54c306085/src/backend/utils/misc/guc-file.l#L653


/* * Escape (by doubling) any single quotes or backslashes in given string **/char*escape_single_quotes_ascii(constchar*src)

https://github.com/postgres/postgres/blob/1acf10549e64c6a52ced570d712fcba1a2f5d1ec/src/port/quotes.c#L33

- we do not touch existing values- escaping of '\n', '\r', '\t', '\b' and '\\' is added- translation of bool into 'on|off' is addedtest_set_auto_conf is updated.
Let's control a correct usage of this function.Plus one assert was added in _escape_config_value.
@demonolock
Copy link
ContributorAuthor

Ошибки flake8
./testgres/node.py:1637:20: E721 do not compare types, for exact checks useis /is not, for instance checks useisinstance()
./testgres/node.py:1702:16: E721 do not compare types, for exact checks useis /is not, for instance checks useisinstance()

dmitry-lipetsk reacted with laugh emoji

@dmitry-lipetsk
Copy link
Collaborator

dmitry-lipetsk commentedDec 3, 2024
edited
Loading

flake8 error:

./build/lib/testgres/node.py:1637:20: E721 do not compare types, for exact checks useis /is not, for instance checks useisinstance()

It is a really interesting problem.

Flake does not like:

asserttype(option)==str

but doesn't mind it:

ifvalueType==str:

For me - it need to configure flake and disable E721.

https://stackoverflow.com/questions/52395064/e721-do-not-compare-types-use-isinstance-error

you should just ignore the linter because it's not suggesting anything useful to you.

In my experience Python linters are blunt tools and implemented fairly naively.

What I do not use isinstance:

Never use isinstance() to validate types, one example is that True and False are considered instances of int

@dmitry-lipetsk
Copy link
Collaborator

dmitry-lipetsk commentedDec 3, 2024
edited
Loading

https://www.flake8rules.com/rules/E721.html

A object should be be compared to a type by using isinstance. This is because isinstance can handle subclasses as well.

They are really thinking that know what I want :)

Copy link
Collaborator

@dmitry-lipetskdmitry-lipetsk left a comment

Choose a reason for hiding this comment

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

This patch was tested with pg_probackup2 test system. All is OK.

@dmitry-lipetskdmitry-lipetsk merged commit5e9ecbc intomasterDec 4, 2024
2 checks passed
@demonolockdemonolock deleted the fix-set-auto-conf branchDecember 4, 2024 10:11
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@dmitry-lipetskdmitry-lipetskdmitry-lipetsk approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants
@demonolock@dmitry-lipetsk

[8]ページ先頭

©2009-2025 Movatter.jp