-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[home] Include ability to publish kibana saved objects from add data tutorial #19559
Changes from 11 commits
b0f6537
a63831a
6decd9d
6eaeb6a
c3cd774
23928e0
bc739f5
822627c
7f9c845
c73f6e9
63db6f9
fc2064d
560b4a9
9ff43d3
e5000e3
7bc56e1
232a760
12e29b3
ef0be0e
6175825
cb3f8de
8ea193d
e037c91
e0cb81b
66faeb3
a4ad403
c0e25ad
7cd8938
7e108c0
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 |
---|---|---|
@@ -0,0 +1,131 @@ | ||
/* | ||
* Licensed to Elasticsearch B.V. under one or more contributor | ||
* license agreements. See the NOTICE file distributed with | ||
* this work for additional information regarding copyright | ||
* ownership. Elasticsearch B.V. licenses this file to you under | ||
* the Apache License, Version 2.0 (the "License"); you may | ||
* not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, | ||
* software distributed under the License is distributed on an | ||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
* KIND, either express or implied. See the License for the | ||
* specific language governing permissions and limitations | ||
* under the License. | ||
*/ | ||
|
||
import React, { Fragment } from 'react'; | ||
import PropTypes from 'prop-types'; | ||
|
||
import { | ||
EuiSteps, | ||
EuiFlexGroup, | ||
EuiFlexItem, | ||
EuiText, | ||
EuiButton, | ||
EuiSpacer, | ||
EuiCallOut, | ||
} from '@elastic/eui'; | ||
|
||
export class SavedObjectsInstaller extends React.Component { | ||
state = { | ||
isInstalling: false, | ||
isInstalled: false, | ||
}; | ||
|
||
installSavedObjects = async () => { | ||
this.setState({ | ||
isInstalling: true, | ||
}); | ||
|
||
const resp = await this.props.bulkCreate(this.props.savedObjects, { overwrite: true }); | ||
const errors = resp.savedObjects.filter(savedObjectCreateResult => { | ||
return savedObjectCreateResult.hasOwnProperty('error'); | ||
}); | ||
const hasErrors = errors.length > 0; | ||
|
||
const statusMsg = hasErrors | ||
? `Unable to load kibana saved objects, Error: ${errors[0]}` | ||
: `${this.props.savedObjects.length} saved objects successfully added`; | ||
|
||
this.setState({ | ||
isInstalling: false, | ||
installStatusMsg: statusMsg, | ||
isInstalled: !hasErrors, | ||
}); | ||
} | ||
|
||
renderInstallMessage() { | ||
if (!this.state.installStatusMsg) { | ||
return; | ||
} | ||
|
||
return ( | ||
<EuiCallOut | ||
title={this.state.installStatusMsg} | ||
color={this.state.isInstalled ? 'success' : 'warning'} | ||
/> | ||
); | ||
} | ||
|
||
renderInstallStep = () => { | ||
const installStep = ( | ||
<Fragment> | ||
<EuiFlexGroup justifyContent="spaceBetween" alignItems="center"> | ||
<EuiFlexItem> | ||
<EuiText> | ||
<p> | ||
Imports index pattern, visualizations and pre-defined dashboards. | ||
Index pattern is required for some features in the APM UI. | ||
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'm thinking we don't want to call out APM specifically here since any module could use this. Maybe we either nix this sentence or let tutorials customize? |
||
</p> | ||
</EuiText> | ||
</EuiFlexItem> | ||
|
||
<EuiFlexItem | ||
grow={false} | ||
> | ||
<EuiButton | ||
onClick={this.installSavedObjects} | ||
isLoading={this.state.isInstalling} | ||
> | ||
Load/Import Kibana objects | ||
</EuiButton> | ||
</EuiFlexItem> | ||
</EuiFlexGroup> | ||
|
||
<EuiSpacer size="s" /> | ||
|
||
{this.renderInstallMessage()} | ||
</Fragment> | ||
); | ||
|
||
return { | ||
title: 'Kibana objects', | ||
status: this.state.isInstalled ? 'complete' : 'incomplete', | ||
children: installStep, | ||
key: 'installStep' | ||
}; | ||
} | ||
|
||
render() { | ||
return ( | ||
<EuiSteps | ||
steps={[this.renderInstallStep()]} | ||
/> | ||
); | ||
} | ||
} | ||
|
||
const savedObjectShape = PropTypes.shape({ | ||
id: PropTypes.string.isRequired, | ||
type: PropTypes.string.isRequired, | ||
attributes: PropTypes.object.isRequired, | ||
}); | ||
|
||
SavedObjectsInstaller.propTypes = { | ||
bulkCreate: PropTypes.func.isRequired, | ||
savedObjects: PropTypes.arrayOf(savedObjectShape).isRequired, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,7 @@ import chrome from 'ui/chrome'; | |
import routes from 'ui/routes'; | ||
import template from './home_ng_wrapper.html'; | ||
import { FeatureCatalogueRegistryProvider } from 'ui/registry/feature_catalogue'; | ||
import { SavedObjectsClientProvider } from 'ui/saved_objects'; | ||
import { uiModules } from 'ui/modules'; | ||
import { | ||
HomeApp | ||
|
@@ -37,6 +38,7 @@ function getRoute() { | |
return { | ||
template, | ||
controller($scope, config, indexPatterns, Private) { | ||
const savedObjectsClient = Private(SavedObjectsClientProvider); | ||
$scope.addBasePath = chrome.addBasePath; | ||
$scope.directories = Private(FeatureCatalogueRegistryProvider).inTitleOrder; | ||
$scope.recentlyAccessed = recentlyAccessed.get().map(item => { | ||
|
@@ -49,6 +51,7 @@ function getRoute() { | |
const getter = indexPatterns.getIds; | ||
getter.clearCache(); | ||
}; | ||
$scope.bulkCreate = savedObjectsClient.bulkCreate; | ||
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 think we made a mistake by coupling rest clients to Angular in the past. Instead they should have been framework-agnostic static functions. If we keep coupling new code (in this case React) to Angular we make it harder to switch away from Angular, and increase technical debt. It's easier to use an existing client but I don't think the alternative is much more difficult. You don't have to re-implement the entire savedObjectsClient - just create a starting point with the endpoints you need. Afaict this is all you need: export function bulkCreate(savedObjects, { overwrite }) {
return kfetch({
pathname: `/api/saved_objects/_bulk_create`,
method: 'POST',
query: {
overwrite
},
body: JSON.stringify(savedObjects)
});
} 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. @epixa Do we already have a strategy for consuming Angular services from outside Angular - and how to migrate them? I've already spent a fair amount of time de-angularizing some services and it would be beneficial if everyone were aligned. If we don't have anything I'm open for doing a write-up of the approach I've used, and how I see us moving forward. 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 do not think this is a problem and is exactly how we are interacting with saved objects in other react components. From react's perspective, it is just an injected function. Eventually, the savedObjectClient will be decoupled from angular but all saved object interaction should go through the savedObjectClient. 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.
Maybe I don't know enough about the 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. The savedObjectClient converts the results into savedObject classes and camel cases the object keys. But more importantly, it puts all calls to the saved object API in a single place so if the API ever changes, only the savedObjectClient has to be updated instead of hunting down lots of segregated fetches. 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.
Totally agree. IMHO it is okay temporarily to have two "clients": the old Angular client, and a new framework agnostic service. Over time the Angular client could even call the framework agnostic client, and eventually the Angular client would be dropped. That's just my view, but interested to hear what other ideas are floating around (ping @epixa ;) ) 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. Btw. I think it's fine to leave it as is. Just my opinion on how to do it going forward. 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. Not in this PR, but I'd rather just remove the angular dependency from the saved object client we have today. It shouldn't be terribly difficult, we'd just need to update any references to the saved object client from angular code to invoke $scope.$apply() after the call. |
||
} | ||
}; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
/* | ||
* Licensed to Elasticsearch B.V. under one or more contributor | ||
* license agreements. See the NOTICE file distributed with | ||
* this work for additional information regarding copyright | ||
* ownership. Elasticsearch B.V. licenses this file to you under | ||
* the Apache License, Version 2.0 (the "License"); you may | ||
* not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, | ||
* software distributed under the License is distributed on an | ||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
* KIND, either express or implied. See the License for the | ||
* specific language governing permissions and limitations | ||
* under the License. | ||
*/ | ||
|
||
import indexPattern from './index_pattern.json'; | ||
import staticSavedObjects from './saved_objects.json'; | ||
|
||
function getIndexPattern(server) { | ||
let apmIndexPattern = 'apm*'; | ||
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 hard code this value in three places now. Maybe time to refactor? Also... it seems like we should never get here if the APM plugin isn't running? The only way to stop it from running is to set it to disabled in kibana.yml, right? In that case I can't even see these instructions. In which case - why swallow the error if that flow isn't expected? I don't think this could happen because I think disabling the plugin requires a restart of kibana, but if there was another way to get this to fail, it seems like it could be an error if there was a custom index name specified, but here the config service was temporarily down, so it falls back to If we should keep it in, is there any way to verify that the error is the one you want to skip? Otherwise we are potentially swallowing other errors and this might lead to hard to debug issues. 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. The APM manifest gets loaded regardless of whether the APM app is running or not. I will try to add a check for the "not found error" to allow other errors to surface. |
||
try { | ||
apmIndexPattern = server.config().get('xpack.apm.indexPattern'); | ||
} catch (error) { | ||
// ignore error when config does not contain 'xpack.apm.indexPattern'. | ||
// This is expected when APM plugin is not running. | ||
} | ||
|
||
indexPattern.attributes.title = apmIndexPattern; | ||
|
||
return indexPattern; | ||
} | ||
|
||
export function getSavedObjects(server) { | ||
return [getIndexPattern(server), ...staticSavedObjects]; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
/* | ||
* Licensed to Elasticsearch B.V. under one or more contributor | ||
* license agreements. See the NOTICE file distributed with | ||
* this work for additional information regarding copyright | ||
* ownership. Elasticsearch B.V. licenses this file to you under | ||
* the Apache License, Version 2.0 (the "License"); you may | ||
* not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, | ||
* software distributed under the License is distributed on an | ||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
* KIND, either express or implied. See the License for the | ||
* specific language governing permissions and limitations | ||
* under the License. | ||
*/ | ||
|
||
import expect from 'expect.js'; | ||
import { getSavedObjects } from './get_saved_objects'; | ||
|
||
const indexPatternTitle = 'dynamic index pattern title'; | ||
|
||
const configMock = { | ||
get: () => { | ||
return indexPatternTitle; | ||
} | ||
}; | ||
|
||
const serverMock = { | ||
config: () => { return configMock; } | ||
}; | ||
|
||
test('should dynamically set index title to "xpack.apm.indexPattern" yaml config value', () => { | ||
const savedObjects = getSavedObjects(serverMock); | ||
const indexPattern = savedObjects[0]; | ||
expect(indexPattern.type).to.be('index-pattern'); | ||
// if index pattern id changes, ensure other saved objects point to the new id | ||
expect(indexPattern.id).to.be('12e52550-6354-11e8-9d01-ed6a4badd083'); | ||
expect(indexPattern.attributes.title).to.be(indexPatternTitle); | ||
}); |
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.
Need to use the isMounted check as this component may have been unmounted due to the async call: