Skip to content

Change plrust.allowed_dependencies to sighup context #362

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.

Sign up for GitHub

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

Jump to bottom
Merged
eeeebbbbrrrr merged 2 commits into from
Jul 21, 2023
Merged

Change plrust.allowed_dependencies to sighup context #362

eeeebbbbrrrr merged 2 commits into from
Jul 21, 2023

Conversation

@anth0nyleung
Copy link
Contributor

@anth0nyleung anth0nyleung commented Jul 19, 2023 •
edited
Loading

Since there is no technical reason l to set plrust.allowed_dependencies to Postmaster context which requires engine restart to change its value, we can change its context to sighup such that the guc can be modified dynamically to provide more flexibility when using allowlisted crates.

Testing

  • Origin value is /tmp/old.toml
(23-07-19 16:04:25) [~]
$ > psql -d postgres -c "show plrust.allowed_dependencies"
 plrust.allowed_dependencies
-----------------------------
 /tmp/old.toml
(1 row)
  • Change value to /tmp/new.toml
(23-07-19 16:04:27) [~]
$ > vim pg_data/postgresql.conf

(23-07-19 16:04:37) [~]
$ > cat pg_data/postgresql.conf | grep plrust.allowed_dependencies
plrust.allowed_dependencies = '/tmp/new.toml'
  • Reload engine and new value is applied
(23-07-19 16:04:43) [~]
$ > psql -d postgres -c "select pg_reload_conf()"
 pg_reload_conf
----------------
 t
(1 row)

(23-07-19 16:04:45) [~]
$ > psql -d postgres -c "show plrust.allowed_dependencies"
 plrust.allowed_dependencies
-----------------------------
 /tmp/new.toml
(1 row)

@eeeebbbbrrrr
Copy link
Contributor

eeeebbbbrrrr commented Jul 21, 2023

This looks good but does require documentation updates. @anth0nyleung if you don't have time/desire to also do those, that's fine -- we'll take care of it after we merge. I'm really noting it so that it doesn't get forgotten.

@anth0nyleung
Copy link
Contributor Author

anth0nyleung commented Jul 21, 2023

@eeeebbbbrrrr Thanks for review. I have also updated the documentation for the guc. Please feel free to edit further if necessary.

@eeeebbbbrrrr eeeebbbbrrrr merged commit c0b5970 into tcdi:develop Jul 21, 2023
@anth0nyleung anth0nyleung deleted the allowed_dependencies_sighup branch August 10, 2023 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@eeeebbbbrrrr eeeebbbbrrrr eeeebbbbrrrr approved these changes

Reviewers whose approvals may not affect merge requirements

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

@anth0nyleung @eeeebbbbrrrr