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

Convert old rest stack: base xdmod #73

Merged
merged 1 commit into from
Apr 11, 2017

Conversation

jsperhac
Copy link
Contributor

@jsperhac jsperhac commented Mar 22, 2017

This is a step along the way toward killing off the old REST stack.

Remove entirely-unused source code from the old classes/REST rest stack. Remove the old documentation, html/rest_catalog/, that described these unused sources. This is a tidier implementation of my old pull request #422 from the xdmod-private ccr repo.

Refer to pull requests against xdmod-appkernel (ubccr/xdmod-appkernels#12) and xdmod-supremm (ubccr/xdmod-supremm#28) repositories that take this pull request further and convert the last remaining old REST stack remnants to use the NewRest controllers. Following the implementation of those pull requests we can remove the old REST stack entirely.

Description

Deleted 28 unused files and a number of directories:

  • classes/REST/Authentication/*
  • classes/REST/Charting/*
  • classes/REST/DataWarehouse/*
  • classes/REST/Portal/*
  • classes/REST/Reporting/*
  • classes/REST/Version/*
  • classes/REST/raw_formats/*
  • classes/iBinaryFormat.php
  • html/rest_catalog/*

Motivation and Context

The motivation behind this work is to:

  • Head in the direction of reducing 3 rest stacks to 2 rest stacks
  • Also: I'm tired of dead, duplicated, unused code.

Tests performed

Front-end was tested by hand in grotesque detail by submitter and by Bob, in November 2016, as part of old and rescinded pull request #422.

Internal dashboard was tested by hand using chrome debugger to observe which rest endpoints are in use for the dashboard functionality. Integration tests were introduced and make use of the integration test harness, for the supremm and appkernel portions.

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.

Caveats:

More things I'd like to do related to the rest stacks:

  • Introduce improved logging for the NewRest controllers

To Do:

Remaining to do in this exercise: see pull requests named "convert-old-rest-stack" for appkernel and supremm modules.

@@ -179,21 +174,14 @@ public static function getInstance()
$app->mount($unversionedPathMountPoint, $controller);
}

// SETUP: an error handler that will let us fall back to the old rest
// stack / controllers.
// SETUP: error handler
$app->error(function (\Exception $e, $code) use ($app) {
Copy link
Contributor

Choose a reason for hiding this comment

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

do we even need this anymore? Since we are not actually trying to fallback, what happens if this is just removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see lots of test errors on removal, so let's keep that code...

Remove legacy router code for old rest endpoints; remove extrest
directory and remove old functions from libraries/rest code.
@jsperhac jsperhac merged commit 3044916 into ubccr:xdmod6.6 Apr 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants