Rename Tables and some changes for Showback Models #96
Conversation
|
@miq-bot add_label wip |
|
We said to keep the models simple, but this would be too simple. Do you think we should use showback_[model]?, because chargeback_[model] is currently in use, and consumption_[model] seems to be confusing |
|
Chargeback_rates exists...so showback_ |
|
This will break tests until the PR for consumption updating the models is merged. |
|
|
||
| rename_column :input_measures, :category, :entity | ||
| rename_column :input_measures, :dimensions, :fields | ||
| rename_column :input_measures, :measure, :group |
There was a problem hiding this comment.
Please add column :start_time and end_time to data_views, so we can store the corresponding information from the event.
A column with context too jsonb
aljesusg
force-pushed
the
showback_chargeback_changes
branch
from
9a32e5a to
e4bda01
Compare
|
|
||
| rename_column :showback_data_views, :stored_data, :snapshot_data | ||
|
|
||
| add_column :showback_data_views, :snapshot_context, :string |
db/schema.yml
Outdated
| - showback_data_rollup_id | ||
| - created_at | ||
| - updated_at | ||
| - snapshot_data |
aljesusg
force-pushed
the
showback_chargeback_changes
branch
from
e4bda01 to
782ba3d
Compare
|
Changed @chargio |
|
@chargio unrecognized command 'review', ignoring... Accepted commands are: add_label, assign, close_issue, move_issue, remove_label, rm_label, set_milestone |
2 similar comments
|
@chargio unrecognized command 'review', ignoring... Accepted commands are: add_label, assign, close_issue, move_issue, remove_label, rm_label, set_milestone |
|
@chargio unrecognized command 'review', ignoring... Accepted commands are: add_label, assign, close_issue, move_issue, remove_label, rm_label, set_milestone |
aljesusg
force-pushed
the
showback_chargeback_changes
branch
from
782ba3d to
ed68ae6
Compare
|
I pushed again @chargio context_snapshot was with string instead jsonb |
|
Checked commit aljesusg@ed68ae6 with ruby 2.3.3, rubocop 0.47.1, and haml-lint 0.20.0 db/migrate/20171018081206_showback_fix_and_rename.rb
|
|
@gtanzillo Could you comment? |
|
Will this require data migrations? |
|
No @Fryguy chargeback is only using engine of consumption, there is not data migrated yet from old chargeback to the new one. |
|
@Fryguy precisely we want to make the name change before we have the first data here... Easier to maintain |
|
|
||
| rename_column :showback_data_views, :stored_data, :data_snapshot | ||
|
|
||
| add_column :showback_data_views, :context_snapshot, :jsonb |
There was a problem hiding this comment.
I'm not sure why these cols are being added. @aljesusg can you comment?
There was a problem hiding this comment.
We need to store the context in the dataview, because formerly we were using it in the event, but our aim is to avoid having to have an event at all. Thus, we need to store the context in the data view too
|
@Fryguy I believe we can merge it, we need to make a couple of PR before next week as this will break consumption |
|
@chargio this will break consumption repo ? If so, I guess it will also break main repo. |
|
@lpichler If you change all names in repos it will break whatever is using them. We should have a PR prepared for that |
aljesusg
changed the title
|
@miq-bot remove_label wip |
|
@lpichler @gtanzillo Please review this PR and the PR that comes together with this. @Fryguy That should cover everything, we should have from now on the proper names in tables and models. |
There was a problem hiding this comment.
LGTM. Since you asked for @lpichler and @gtanzillo I'll wait for their approval as well before merge.
Change name to tables and rename some columns .
@chargio Please review this . Maybe I am missing something.
In Usagetypes dimension is plural because this will be array so will be dimensions not dimension like rate, ok?
Now working in the PR for https://github.com/ManageIQ/manageiq-consumption changes wait until this please.