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

[DRAFT] Update to ActiveRecord 6 #16

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

[DRAFT] Update to ActiveRecord 6 #16

wants to merge 5 commits into from

Conversation

amotl
Copy link
Member

@amotl amotl commented Nov 28, 2022

The goal is to update dependencies and code to be compatible with ActiveRecord 6, which is the last version compatible with Ruby 2.6. Updating to ActiveRecord 7, and dropping support for older Ruby versions, can be done in a subsequent patch.

@amotl amotl force-pushed the collab/refresh branch 4 times, most recently from 3c3183a to b26adff Compare November 29, 2022 13:04
@amotl amotl changed the title Update to ActiveRecord 7 and crate_ruby 0.2 Update to ActiveRecord 6/7 and crate_ruby 0.2 Nov 29, 2022
@amotl amotl changed the title Update to ActiveRecord 6/7 and crate_ruby 0.2 Update to ActiveRecord 6 and crate_ruby 0.2 Nov 29, 2022
@amotl amotl changed the title Update to ActiveRecord 6 and crate_ruby 0.2 [WIP] Update to ActiveRecord 6 and crate_ruby 0.2 Nov 29, 2022
Comment on lines 143 to 149
def columns(table_name) #:nodoc:
cols = @connection.table_structure(table_name).map do |field|
name = dotted_name(field[2])
# TODO: Review. What if the table structure changes again?
name = dotted_name(field[12])
CrateColumn.new(name, nil, field[4], nil)
end
cols
Copy link
Member Author

@amotl amotl Nov 29, 2022

Choose a reason for hiding this comment

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

This feels a bit fishy, and I don't even know if it is correct. See also CrateRuby::Client::table_structure, where select * from information_schema.columns is used, which, when used in combination, is prone to break sooner than later.

For inquiring the table columns, something more robust along the lines of sqlalchemy/dialect.py#L251-L263 should be used instead.

@amotl amotl changed the title [WIP] Update to ActiveRecord 6 and crate_ruby 0.2 [DRAFT] Update to ActiveRecord 6 Nov 30, 2022
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.

2 participants