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

Suggestions #63

Open
fatkodima opened this issue May 22, 2021 · 8 comments
Open

Suggestions #63

fatkodima opened this issue May 22, 2021 · 8 comments

Comments

@fatkodima
Copy link
Contributor

  1. Currently, all tables are checked against some violations. But for me, it makes more sense to check only tables having a model within application. When the db schema is not kept in the cleanest state, it is possible to have ununsed tables and to get false positives on them.
  2. This gem does not support rails multiple databases feature.
  3. There are some steps in the README on how to create migrations, but it would be helpful to be able to generate migrations automatically (by providing some flag to the rake task), when it is appropriate

Would be happy to help with PRs, if this is something you are looking for.

@fatkodima
Copy link
Contributor Author

@gregnavis, So I'm considering to implement 2-nd point. Is this something you want to this in this gem?

And regarding 1-st point: it will greatly simplify the implementation of 2-nd, since we can basically iterate over models and use their #connection to retrieve all we needed instead of manually try to connect to different databases and iterate over tables/views/indexes/etc there.

@gregnavis
Copy link
Owner

Re 1, I'm thinking about adding a global option to make this possible. What do you think?

Re 2, what kind of features were you thinking about to support this?

@fatkodima
Copy link
Contributor Author

Re 1, I'm thinking about adding a global option to make this possible. What do you think?

👍

Re 2, what kind of features were you thinking about to support this?

I'm worrying I can misinterpret this question, so I would appreciate if you would rephrase it.

@gregnavis
Copy link
Owner

Re 2, how could active_record_doctor help there? I can imagine ensuring databases specified in connects_to are actually defined in databases.yml. What other checks were you thinking about?

@fatkodima
Copy link
Contributor Author

I mean not new checks (detectors) somehow related to multiple databases. But be able to run existing checks against other databases.

For example, if I have primary and analytics databases, I will want to run ExtraneousIndexes detector against both of them.

@gregnavis
Copy link
Owner

@fatkodima, shouldn't both database use the same schema?

@fatkodima
Copy link
Contributor Author

For example, primary database contains users and projects, but analytics contains clicks, downloads and similar things.

@vinnyFD
Copy link

vinnyFD commented Mar 12, 2023

@gregnavis To your point, db/migrate/ and db/analytics_migrate/ would create 2 schema files, schema.rb and analytics_schema.rb . It would be great if active_record_doctor could detect the analytics migrations. Currently, the tables need to be added to global :ignore_tables in .active_record_doctor file, to avoid "relation "analytics_clicks" does not exist" for example . CC @fatkodima

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

No branches or pull requests

3 participants