Use URL instead authentication and connectivity options for PostgreSQL #837
Conversation
Instead of specifying separte options and credentials:
database PostgreSQL do |db|
db.name = "database_name"
db.username = "username"
db.password = "password"
db.host = "localhost"
db.port = 5432
end
users may user just one -- `url`:
database PostgreSQL do |db|
db.url = "postgres://my_username:password@localhost:5432/my_database_name"
end
|
Any chance to have this soon? @mrrooijen |
|
He doesn't do anything with backup anymore. |
| end | ||
|
|
||
| def pgdump_arguments | ||
| if url.nil? |
There was a problem hiding this comment.
I would turn this if statement around.
if url
# options based on url
else
# username + password method
end| "#{username_option} #{connectivity_options} #{user_options} #{tables_to_dump} " \ | ||
| "#{tables_to_skip} #{name}" | ||
| else | ||
| "#{user_options} #{tables_to_dump} #{tables_to_skip} #{url}" |
There was a problem hiding this comment.
Could we also make this options string like this?
That way we don't have to duplicate certain options in both strings.
args = "#{tables_to_dump} #{tables_to_skip} #{user_options}"
if url
args << " #{url}"
else
args << " #{username_option} #{connectivity_options} #{name}"
end
args| expect(db.send(:pgdump)).to eq( | ||
| "pg_dump #{" " * (option_methods.count - 1)}" | ||
| ) | ||
| context "when url is set" do |
| ) | ||
| end | ||
| # password_option and sudo_option leave no leading space if it's not used | ||
| context "when url is not set" do |
There was a problem hiding this comment.
when username and password options are set
| expect(db.send(:pgdump_arguments)).to eq "#{option_methods.join(" ")} #{db.url}" | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
I don't like how it's testing a private method and stubbing all the option methods. But since that's how the rest of this spec is set up I'll allow it.
| @@ -22,6 +22,10 @@ class Error < Backup::Error; end | |||
| attr_accessor :host, :port, :socket | |||
|
|
|||
| ## | |||
| # URL includes credentials and connectivity options | |||
| attr_accessor :url | |||
There was a problem hiding this comment.
we should probably also output a warning when this option is used in combination with the username, password, name, host, port, socket, etc options, otherwise people might use both while most options are ignored when the url option is used.
Should include some testing for that as well
For example:
def check_options
return unless url
ignored_options = {
username: username,
password: password,
host: host,
port: port,
socket: socket,
name: name
}.reject { |_, value| value.nil? }
return if ignored_options.empty?
Logger.warn "PostgreSQL: the options " \
"#{ignored_options.keys.join(", ")} are set, but will be " \
"ignored because the `:url` option is set."
endand call this method in the perform method, or in the initializer.
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Instead of specifying separte options and credentials:
users may user just one --
url:Fixes backup/backup-features#39