Skip to content
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

Change plrust.allowed_dependencies to sighup context #362

Merged
merged 2 commits into from
Jul 21, 2023

Conversation

anth0nyleung
Copy link
Contributor

@anth0nyleung anth0nyleung commented Jul 19, 2023

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
Collaborator

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

@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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants