-
Notifications
You must be signed in to change notification settings - Fork 68
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
Refactoring of ETL/DbEntity into ETL/DbModel #138
Conversation
@@ -1,3 +1,5 @@ | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like some extra lines ended up here you dont need.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM after removing leading new lines in tools/etl/etl_table_manager.php
This enhancement breaks existing tests that are found in open_xdmod/modules/xdmod/tests/lib/ETL. These tests should be replaced or fixed. |
@jsperhac Did you run |
Ah, sounds like my repo scripts are not accounting for the test artifacts. Let me try... |
* Added tests for DbEntity\Table * Moved namespace ETL\DbEntity to ETL\DbModel * Refactoring of ETL/DbModel to remove old code and support upcoming features * Updated commit for xdmod-test-artifacts * Fix record formula verification and saving of overseer restriction value * Add @plessbd blacklist filters * Ignore case where temp directory already exists for tests
This PR refactors the existing ETL/DbEntity code into a new namespace (ETL/DbModel) and also removes cruft that was not needed and makes it easier to extend. This work is in preparation for supporting improvements such as better subquery support, programmatic changes to queries and tables during ETL, and renaming columns as opposed to dropping/adding them.
See also PR ubccr/xdmod-xsede#32 for additional required changes.
Note that commit
533b2f3
clearly shows several files as renamed and0969e8f
shows many of the changes to those files but the code shown at the end of the PR shows them deleted and recreated. I'm not sure why this happens.Description
The
ETL\DbEntity
code was largely untouched since inception 1.5 years ago and needed to be updated to support desirable features such as subqueries, change column, and improved programmatic access and modification of tables and queries during ETL (for batch aggregation speed improvements, for example).The newly refactored
ETL/DbModel
code has been updated to continue to support a lightweight representation of database queries and tables.The following changes have been made:
ETL/DBModel
protected
properties array inDbEntity.php
and accessed via__get()
,__set()
, and__isset()
. This means that classes no longer need to implement getters in most cases and only need to extend__set()
for complex data types such as arrays of objects where a particular type of object must be created.stdClass
object that can then be fed back into itself as the configuration object. This allows for programmatic modification of an object on the fly during ETL.Table::discover()
method no longer needed to be static.DbEntity
and classes that extend this class now only implement the definition, verification, and setting of the properties that they define.stdClass
objectsstdClass
representation, and feeding this back to ensure the same statement is generatedMotivation and Context
General code cleanup and preparation for new features. Also added tests.
Tests performed
Component tests were run (
PHPUnit
) and theresource-allocations
andxdcdb-jobs
pipelines were run with their data being compared to the a baseline from v6.6 data. Baseline commands are shown here for reference and were the same options as branch 6.7 executions.Test
resource-allocations
andxdcdb-jobs
pipelinesVerify data in generated tables. Note that the default values for
submission_venue_id
,job_record_type_id
, andjob_task_type_id
have changed between 6.6 and 6.7 so these columns are ignored in the jobs data (we do not currently populate them other than the default).Types of changes
Checklist: