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

Ensure id and *_id columns are bigint #248

Merged
merged 1 commit into from
Aug 9, 2018

Conversation

bdunne
Copy link
Member

@bdunne bdunne commented Aug 9, 2018

Based on changes in #245

Adding this as an automated code review to address #4 (comment)

@miq-bot miq-bot added the wip label Aug 9, 2018
@bdunne bdunne force-pushed the ensure_id_columns_are_bigint branch from 0f34e78 to c6dd3e0 Compare August 9, 2018 14:34
"network_ports.binding_host_id",
"scan_histories.task_id", # <--WAT?
"sessions.session_id"
].freeze
Copy link
Member

Choose a reason for hiding this comment

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

Minor, but maybe make this a Set, since we are doing continual lookups

@Fryguy
Copy link
Member

Fryguy commented Aug 9, 2018

As discussed, the tests fail and tell the user to remove the column from the exclusion list when they fix it.

Also, the exclusions_list should be a constant like in the other PR>

@bdunne bdunne force-pushed the ensure_id_columns_are_bigint branch 2 times, most recently from 9c8c25f to 264b7d0 Compare August 9, 2018 16:37
@bdunne bdunne changed the title [WIP] Ensure *_id columns are bigint Ensure *_id columns are bigint Aug 9, 2018
@bdunne bdunne closed this Aug 9, 2018
@bdunne bdunne reopened this Aug 9, 2018
@miq-bot miq-bot removed the wip label Aug 9, 2018
@bdunne bdunne force-pushed the ensure_id_columns_are_bigint branch from 264b7d0 to 971e9b5 Compare August 9, 2018 18:07
@miq-bot
Copy link
Member

miq-bot commented Aug 9, 2018

Checked commit bdunne@971e9b5 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 🍪

@bdunne bdunne changed the title Ensure *_id columns are bigint Ensure id and *_id columns are bigint Aug 9, 2018
it "should be type bigint" do
ActiveRecord::Base.connection.tables.each do |table|
next if ManageIQ::Schema::SYSTEM_TABLES.include?(table)
DatabaseHelper.columns_for_table(table).each do |column|
Copy link
Member

Choose a reason for hiding this comment

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

Moving forward, perhaps this should move into a DatabaseHelper method...

def DatabaseHelper.each_table
  ActiveRecord::Base.connection.tables.each do |table|
    next if ManageIQ::Schema::SYSTEM_TABLES.include?(table)
    yield table
  end
end

def DatabaseHelper.each_column
  each_table do |table|
    columns_for_table(table).each do |column|
      yield table, column
    end
  end
end

Then caller just does

DatabaseHelper.each_column do |table, column|
   ...
end

@Fryguy Fryguy merged commit 00d3b57 into ManageIQ:master Aug 9, 2018
@Fryguy Fryguy added this to the Sprint 92 Ending Aug 13, 2018 milestone Aug 9, 2018
@bdunne bdunne deleted the ensure_id_columns_are_bigint branch August 9, 2018 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants