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

Refactor Accounts Realm #50

Merged
merged 5 commits into from
Feb 11, 2017
Merged

Refactor Accounts Realm #50

merged 5 commits into from
Feb 11, 2017

Conversation

smgallo
Copy link
Contributor

@smgallo smgallo commented Feb 8, 2017

This is related to ubccr/xdmod-xsede#8 as well as https://tas-tools-int-01.ccr.xdmod.org/git/UBCCR/ccr-private-xdmod/pulls/6

Description

This PR includes minor debug improvements to display the DataEndpoint used in each query and removes the min_activity_time column from the peopleonaccount table since it is no longer used.

Motivation and Context

To provide the most accurate information possible.

Tests performed

See ubccr/xdmod-xsede#8

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.

@smgallo smgallo requested a review from jtpalmer February 8, 2017 19:47
ALTER TABLE `peopleonaccount` ADD KEY `aggregation_index` (`resource_id`,`account_id`,`person_id`,`start_time_ts`,`end_time_ts`,`allocationstate_id`);

ALTER TABLE `peopleonaccount` ENABLE KEYS;
UNLOCK TABLES;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add INSERT INTO schema_version_history VALUES ('modw', '6.6.0', NOW(), 'upgraded', 'N/A'); at the end of this file. Also, please update the schema version in db/data/modw.sql.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the schema version to 6.6.0 in db/data/modw.sql:
INSERT INTO schema_version_history VALUES ('modw', '6.6.0', NOW(), 'created', 'N/A');

@@ -45,12 +39,10 @@ function __construct($dest_db, $src_db, $start_date = '1997-01-01', $end_date =
'resource_id',
'person_id',
'allocationstate_id',
'min_activity_time',
'start_time',
'end_time',
'comments',
)
);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this file being used by ETLv2? It doesn't appear to be used anywhere in Open XDMoD. I tried adding it to the list of ingestors (in the old-style ETL), but get this error:

SQLSTATE[42S22]: Column not found: 1054 Unknown column 'poah.state_id' in 'field list'

... which isn't even related to your changes here. This looks like one of those classes that was copy/pasted from the TGcDB ingestors, but we aren't using it since we do not yet have the corresponding data in Open XDMoD.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This isn't used in ETLv2 at all. If we're not using it in Open XDMoD then I'll remove it from the codebase and we can deal with it when we make modifications to support allocations/projects.

@smgallo
Copy link
Contributor Author

smgallo commented Feb 10, 2017

Addressed @jtpalmer comments

@smgallo smgallo merged commit d05d568 into ubccr:xdmod6.6 Feb 11, 2017
@smgallo smgallo deleted the accounts-realm branch February 11, 2017 13:39
@tyearke tyearke added enhancement Enhancement of the functionality of an existing feature qa labels Mar 8, 2017
@tyearke tyearke added this to the v6.6.0 milestone Mar 8, 2017
ryanrath pushed a commit to ryanrath/xdmod that referenced this pull request Apr 27, 2017
* Minor improvements to ETLv2 logging
* Remove unused min_activity_time column from peopleonaccount table
* Add schema migration file
* Updated schema versions and removed peopleonaccount ingestor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement of the functionality of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants