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

Allow more database.yml settings #1194

Open
dmaes opened this issue Nov 13, 2024 · 5 comments · May be fixed by #1195
Open

Allow more database.yml settings #1194

dmaes opened this issue Nov 13, 2024 · 5 comments · May be fixed by #1195

Comments

@dmaes
Copy link

dmaes commented Nov 13, 2024

We need to be able to set the target_session_attrs parameter in database.yml (which is a connection parameter for libpq), but this is currently not supported. I'm open to making a PR, but unsure if there is a preference for adding a String $db_target_session_attrs parameter, or just a generic Hash[String, String] $db_extra_options parameter, which we can then use to set any option.
I am aware that target_session_attrs is not documented in foreman or rails docs, but from https://github.com/rails/rails/blob/main/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb#L47 , any parameter not documented there, will be passed as libpq connection parameter.

@ekohl ekohl changed the title Allow more more database.yml settings Allow more database.yml settings Nov 13, 2024
@ekohl
Copy link
Member

ekohl commented Nov 13, 2024

target_session_attrs

For my own interest: https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNECT-TARGET-SESSION-ATTRS

I'm open to making a PR, but unsure if there is a preference for adding a String $db_target_session_attrs parameter, or just a generic Hash[String, String] $db_extra_options parameter, which we can then use to set any option.

Something I've been thinking about recently is just exposing a database URL parameter instead of each component. That gives a lot more freedom for these things. However, it would have a lot more consequences like needing to write migrations and properly test it so I haven't started it.

Any thoughts on this matter?

@dmaes
Copy link
Author

dmaes commented Nov 13, 2024

I think parameter for the "default" things (host,port,username,password,etc...) definitely has it's worth for simplicity's sake for most setups. Hence why I suggested a Hash $db_extra_options, which could be somehting like the following in epp:

<% $extra_options.each |$k, $v| { -%>
<%= $k %>: <%= $v %>
<% } -%>

or even just a String $db_extra_fragment to just <%= $extra_fragment %> at the end of the template, although the first option is probably a bit cleaner, maybe even with a to_yaml and some indentation prefixing, instead of a .each.

@ekohl
Copy link
Member

ekohl commented Nov 13, 2024

I'm slightly more of a fan of Hash $db_extra_options so a PR to add that is IMHO a good addition.

@dmaes dmaes linked a pull request Nov 13, 2024 that will close this issue
@dmaes
Copy link
Author

dmaes commented Nov 13, 2024

Added a PR with Hash[String, String] $db_extra_options.

@dmaes
Copy link
Author

dmaes commented Nov 20, 2024

@ekohl anything missing in the PR for it to get reviewed & merged?

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 a pull request may close this issue.

2 participants