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

Change staging table to use 1 as unknown id instead of -1 #866

Merged

Conversation

eiffel777
Copy link
Contributor

For columns in the cloud staging tables related to the changes in #861 change the COALESCE statements to use 1 instead of -1.

Tests performed

Ran all tests in docker

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project as found in the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@eiffel777 eiffel777 added the Category:Cloud Cloud Realm label Mar 21, 2019
@eiffel777 eiffel777 added this to the 8.1.0 milestone Mar 21, 2019
@eiffel777 eiffel777 self-assigned this Mar 21, 2019
chakrabortyr
chakrabortyr previously approved these changes Mar 21, 2019
@jpwhite4
Copy link
Member

jpwhite4 commented Mar 21, 2019

Did the pre-populated table data change too?
[edit] I see that the pre-population action was already setting 1 as the unknown

@jpwhite4 jpwhite4 closed this Mar 21, 2019
@jpwhite4 jpwhite4 reopened this Mar 21, 2019
Copy link
Member

@jpwhite4 jpwhite4 left a comment

Choose a reason for hiding this comment

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

Where is the prepopulated data for these tables that have 'Unknown' hardcoded? I can't see it in the repo.
[edit] I see that it is set in the initialize_unknown_dimension_values action

@jpwhite4
Copy link
Member

In the docs/cloud.md is says to run

php /usr/share/xdmod/tools/etl/etl_overseer.php -p jobs-common -p jobs-cloud-common -p ingest-resources

But I think that the jobs-cloud-common must be run after the ingest-resources otherwise the 'unknown' rows will not be populated properly.

@jpwhite4
Copy link
Member

Need to update the comment for account_id column in configuration/etl/etl_tables.d/cloud_common/account.json

@jpwhite4
Copy link
Member

Should not set the default person_id to 1 in configuration/etl/etl_tables.d/cloud_common/instance.json

@jpwhite4
Copy link
Member

The comment on the modw_cloud.region.region_id column states

"Auto-increment relative to resource_id. We do not have unknown assets."

yet this table is populated with an 'unknown' row by the initialize_unknown_dimension_values.json. Which is correct?

@eiffel777 eiffel777 merged commit 9cef334 into ubccr:xdmod8.1 Mar 22, 2019
@eiffel777 eiffel777 added the bug Bugfixes label Mar 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bugfixes Category:Cloud Cloud Realm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants