Skip to content

added :ignore_database_name option#357

Merged
drwl merged 1 commit into
drwl:mainfrom
persona-id:ignore-database-name
Jul 14, 2026
Merged

added :ignore_database_name option#357
drwl merged 1 commit into
drwl:mainfrom
persona-id:ignore-database-name

Conversation

@HoneyryderChuck

Copy link
Copy Markdown
Contributor

when enabled, it won't write the database name to the annotatino block

this option may be confusing in a sharded database scenario, where the development name just does not map to reality

when enabled, it won't write the database name to the annotatino block

this option may be confusing in a sharded database scenario, where the development name just does not map to reality

@drwl drwl left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for the PR and the explanation 👍

@drwl
drwl merged commit 2abb99e into drwl:main Jul 14, 2026
32 checks passed
@OdenTakashi

OdenTakashi commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Thank you for the PR! Sorry for the delayed reply.

Is this for a multi-db environment? If so, there is already an ignore_multi_database_name option for this. ( #283 )
That said, it defaults to false. Originally there was no option to ignore the database name, so we kept the default as false to avoid changing the existing behavior. You'll need to set it to true for it to take effect.
If ignore_multi_database_name covers your use case, I'd like to remove this new option since it would be redundant.

( Also, having an option that isn't very visible / is easy to miss feels like something we should fix. )

@HoneyryderChuck
HoneyryderChuck deleted the ignore-database-name branch July 14, 2026 08:29
@HoneyryderChuck

Copy link
Copy Markdown
Contributor Author

@OdenTakashi our setup is not for multi database, just sharded. I had tried ignore_multi_database_name support before, and it didn't work either for our case (Database Name is still there).

I also think that it's quite common to name your database differently per environment ("${name}_production" for example), so having it pop up in annotations gets confusing, hence why I think this option is justified. We were previously using annotate gem before, and had to change to annotaterb in the process of upgrading to rails 8 (which annotate no longer supports), and this was the only gotcha we found (annotate does not write database names).

@OdenTakashi

Copy link
Copy Markdown
Collaborator

@HoneyryderChuck Thanks for the clarification — I overlooked that your setup is sharded rather than multi-database. Sorry about that.

I tried this locally with a sharded configuration and confirmed that the database name is included there as well. In my test, setting ignore_multi_database_name: true suppressed it. The logic that adds database_name only checks whether there are multiple database configurations for the current environment (configs_for(...).size > 1), so a sharded configuration is treated the same way as a multi-database configuration here.

This is also the only place where database_name is added to the annotation data. When ignore_multi_database_name is true, multi_db_environment? returns false, so database_name is not passed through:

database_name = @model.database_name if multi_db_environment?

# ...

def multi_db_environment?
  return false if @options[:ignore_multi_database_name]

  if defined?(::Rails) && ::Rails.env
    ActiveRecord::Base.configurations.configs_for(env_name: ::Rails.env).size > 1
  else
    false
  end
end

That said, the name ignore_multi_database_name does not quite match the sharding use case, which feels a little confusing. This gave me a new perspective on the existing option, so I may take a look at consolidating this behavior under ignore_database_name myself.

Thanks again for the clarification — this was a helpful insight.

@drwl

drwl commented Jul 15, 2026

Copy link
Copy Markdown
Owner

@OdenTakashi Thank you for looking deeper into this. I should have looked at the existing options to see if there was any pre-existing behavior, so thanks for catching that.

@HoneyryderChuck

Copy link
Copy Markdown
Contributor Author

@OdenTakashi just a clarification that the development config ain't sharded, which is probably why the multi db option didn't catch it. And we annotate in development.

@OdenTakashi

OdenTakashi commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator

@HoneyryderChuck
Thanks for the clarification.

I want to make sure I understand what happened correctly: in development, where the configuration uses neither multiple databases nor sharding, was the database name still included in the annotation even with ignore_multi_database_name: true?

I’m curious about what caused that, since ignore_multi_database_name: true should skip the condition that adds the database name to the annotation.

If you have a minimal reproduction, it would be helpful if you could share it.

@HoneyryderChuck

Copy link
Copy Markdown
Contributor Author

@OdenTakashi my bad, turns out the issue was in the config file on my side (: ignore_multi_database_name had 2 entries). I confirm : ignore_multi_database_name works. So yeah, I agree with your earlier statement that only the option name could be made clearer.

@OdenTakashi

OdenTakashi commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

@HoneyryderChuck Thanks for checking and confirming!

That makes sense — I’m glad to hear that ignore_multi_database_name works on your side.

Since ignore_database_name better reflects the actual behavior, including sharded configurations, I think it makes sense to use it as the clearer, preferred option going forward.

Since some users may already rely on ignore_multi_database_name, I’ll look into consolidating the two options while preserving backward compatibility.

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.

3 participants