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

Adding Logging to Query.php #66

Merged
merged 2 commits into from
Mar 7, 2017
Merged

Conversation

ryanrath
Copy link
Contributor

@ryanrath ryanrath commented Mar 7, 2017

Description

  • Added a file logger to Query.php ( query.log )
  • Added logic to the function 'execute' such that, when called and
    'sql_debug_mode' is true, the query string that is about to be executed is logged.

Motivation and Context

It allows some visibility into the queries being generated / executed by the Query class.

Tests performed

  • I deployed the change to my cloud instance ( rr-db1 ), verified there were no issues creating the log file.
  • verified that when 'sql_debug_mode' was set to 'on' queries were logged as expected.
  • verified that when 'sql_debug_mode' was set to 'off' queries were not logged as expected.

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.

- Added a file logger to Query.php ( query.log )
- Added logic to the function 'execute' such that, when called and then
  'sql_debug_mode' is true, the query that is about to be executed is logged.
@@ -229,6 +236,13 @@ public function execute($limit = 10000000)
{

$query_string = $this->getQueryString($limit);

$debug = filter_var(\xd_utilities\getConfiguration('general', 'sql_debug_mode'), FILTER_VALIDATE_BOOLEAN);
Copy link
Contributor

Choose a reason for hiding this comment

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

filter_var is broken in php 5.3 (which is our supported version). See issue #64 for a workaround. I have a function in the ETL\Utilities class but maybe it should be brought up into xd_utilities.

- Added a new 'filter_var' function to 'xd_utilities' that we can use to get
  around the limitations of the 'filter_var' implementation in PHP 5.3.x.
- Updated the 'filter_var' usage in Query to utilize the new function in
  'xd_utilities'.
Copy link
Contributor

@smgallo smgallo left a comment

Choose a reason for hiding this comment

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

LGTM. I'll make a note to remove it from the ETL code and use the xd_utilities version instead.

@ryanrath ryanrath merged commit db0c1b4 into ubccr:xdmod6.6 Mar 7, 2017
@ryanrath ryanrath deleted the query_logging branch March 7, 2017 16:03
@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 added a commit to ryanrath/xdmod that referenced this pull request Apr 27, 2017
* Adding Logging to Query.php

- Added a file logger to Query.php ( query.log )
- Added logic to the function 'execute' such that, when called and then
  'sql_debug_mode' is true, the query that is about to be executed is logged.

* Updated per code review

- Added a new 'filter_var' function to 'xd_utilities' that we can use to get
  around the limitations of the 'filter_var' implementation in PHP 5.3.x.
- Updated the 'filter_var' usage in Query to utilize the new function in
  'xd_utilities'.
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.

3 participants