-
Notifications
You must be signed in to change notification settings - Fork 16
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
Service integration > Nodes Repo Checks page #58
Changes from all commits
dfb2555
d9e5b34
6b189f3
7bd1a63
5f1a511
56b4bc5
cc48972
272b9de
409d6cb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,7 +12,7 @@ | |
angular | ||
.module('crowbarApp', [ | ||
'crowbarCore', | ||
'crowbarData', | ||
'suseData', | ||
'crowbarWidgets', | ||
|
||
/* | ||
|
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
(function () { | ||
'use strict'; | ||
|
||
angular.module('suseData.crowbar', []); | ||
})(); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
/*global bard $httpBackend should expect upgradeBackupFactory assert */ | ||
describe('Upgrade Backup Factory', function () { | ||
/*global bard $httpBackend should expect crowbarBackupFactory assert */ | ||
describe('Crowbar Backup Factory', function () { | ||
|
||
var mockedBackupFile = '--mockedBackupFile--', | ||
mockedCreateResponse = { | ||
|
@@ -9,8 +9,8 @@ describe('Upgrade Backup Factory', function () { | |
|
||
beforeEach(function () { | ||
//Setup the module and dependencies to be used. | ||
bard.appModule('crowbarData.upgrade'); | ||
bard.inject('upgradeBackupFactory', '$q', '$httpBackend'); | ||
bard.appModule('suseData.crowbar'); | ||
bard.inject('crowbarBackupFactory', '$q', '$httpBackend', 'COMMON_API_V2_HEADERS'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we really need to inject There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. According to the API documentation, we do |
||
}); | ||
|
||
afterEach(function() { | ||
|
@@ -22,23 +22,23 @@ describe('Upgrade Backup Factory', function () { | |
describe('when executed', function () { | ||
|
||
it('returns an object', function () { | ||
should.exist(upgradeBackupFactory); | ||
should.exist(crowbarBackupFactory); | ||
}); | ||
|
||
it('returns an object with create function defined', function () { | ||
expect(upgradeBackupFactory.create).toEqual(jasmine.any(Function)); | ||
expect(crowbarBackupFactory.create).toEqual(jasmine.any(Function)); | ||
}); | ||
|
||
it('returns an object with download function defined', function () { | ||
expect(upgradeBackupFactory.download).toEqual(jasmine.any(Function)); | ||
it('returns an object with get function defined', function () { | ||
expect(crowbarBackupFactory.get).toEqual(jasmine.any(Function)); | ||
}); | ||
|
||
describe('when create method is executed', function () { | ||
|
||
beforeEach(function () { | ||
$httpBackend.expect('POST', '/api/crowbar/backups') | ||
$httpBackend.expect('POST', '/api/crowbar/backups', undefined) | ||
.respond(200, mockedCreateResponse); | ||
backupPromise = upgradeBackupFactory.create(); | ||
backupPromise = crowbarBackupFactory.create(); | ||
$httpBackend.flush(); | ||
}); | ||
|
||
|
@@ -65,7 +65,7 @@ describe('Upgrade Backup Factory', function () { | |
beforeEach(function () { | ||
$httpBackend.expect('GET', '/api/crowbar/backups/42/download') | ||
.respond(200, mockedBackupFile); | ||
backupPromise = upgradeBackupFactory.download(42); | ||
backupPromise = crowbarBackupFactory.get(42); | ||
$httpBackend.flush(); | ||
}); | ||
|
||
|
@@ -92,7 +92,7 @@ describe('Upgrade Backup Factory', function () { | |
describe('when download method is executed without parameter', function () { | ||
|
||
it('throws an exception', function () { | ||
expect(upgradeBackupFactory.download).toThrow(); | ||
expect(crowbarBackupFactory.get).toThrow(); | ||
}); | ||
}); | ||
}); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,84 @@ | ||
(function() { | ||
|
||
angular | ||
.module('suseData.crowbar') | ||
.factory('crowbarFactory', crowbarFactory); | ||
|
||
crowbarFactory.$inject = ['$q', '$http', 'COMMON_API_V2_HEADERS']; | ||
/* @ngInject */ | ||
function crowbarFactory($q, $http, COMMON_API_V2_HEADERS) { | ||
var factory = { | ||
getEntity: getEntity, | ||
getRepositoriesChecks: getRepositoriesChecks, | ||
upgrade: upgrade, | ||
getUpgradeStatus: getUpgradeStatus | ||
}; | ||
|
||
return factory; | ||
|
||
/** | ||
* Get Crowbar Entity | ||
* | ||
* @return {Promise} | ||
*/ | ||
function getEntity() { | ||
|
||
var requestOptions = { | ||
method: 'GET', | ||
url: '/api/crowbar', | ||
headers: COMMON_API_V2_HEADERS | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cant we set $http.defaults.headers to avoid adding the headers on each function? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That should be isolated to crowbar endpoints only, so we allow different headers for different endpoints in the future (OpenStack, for instance) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good, anything to avoid repetition 👍 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @skazi0 Would Restangular help us here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would simplify things a bit. We could use "decoupled service" (https://github.com/mgonto/restangular#decoupled-restangular-service) for the /api/ prefix and pre-configure it with required headers and other options if needed like I did here: https://github.com/skazi0/yaia/blob/master/app/static/js/invoices.js#L22-L27 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could do it, but current implementation works fine so I'd leave migration to Restangular for the future or when we're blocked by some problem with direct $http usage. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've created #61 to track it in the future |
||
}; | ||
|
||
return $http(requestOptions); | ||
} | ||
|
||
/** | ||
* Get Administration repositories checks (Operative System and Open Stack) | ||
* | ||
* @return {Promise} | ||
*/ | ||
function getRepositoriesChecks() { | ||
|
||
var requestOptions = { | ||
method: 'GET', | ||
url: '/api/crowbar/repocheck', | ||
headers: COMMON_API_V2_HEADERS | ||
}; | ||
|
||
return $http(requestOptions); | ||
|
||
} | ||
|
||
/** | ||
* Trigger the Upgrade process on the Adinistration Node (Crowbar) from Cloud6 to Cloud7 | ||
* | ||
* @return {Promise} | ||
*/ | ||
function upgrade() { | ||
|
||
var requestOptions = { | ||
method: 'POST', | ||
url: '/api/crowbar/upgrade', | ||
headers: COMMON_API_V2_HEADERS | ||
}; | ||
|
||
return $http(requestOptions); | ||
} | ||
|
||
/** | ||
* Get the upgrade status of the Administration Node (Crowbar) | ||
* | ||
* @return {Promise} | ||
*/ | ||
function getUpgradeStatus() { | ||
|
||
var requestOptions = { | ||
method: 'GET', | ||
url: '/api/crowbar/upgrade', | ||
headers: COMMON_API_V2_HEADERS | ||
}; | ||
|
||
return $http(requestOptions); | ||
} | ||
} | ||
})(); |
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.
create -> createBackup for consistency?
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.
In the end it would be used like:
crowbarBackupFactory().createBackup()
. Don't you think it's redundant, considering we're already operating over a backup factory?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.
Yeah, a bit redundant but as the var+function below changed their names to include backup, I was thinking more about consistency between both methods
crowbarBackupFactory().createBackup()
andcrowbarBackupFactory().getBackup()
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.
I agree with @Itxaka here. It should be either
create
&download
orcreateBackup
&getBackup
ordownloadBackup
.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.
Will be updated