-
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
Adds deprecation notice for defaultAppId in favor of defaultRoute #11590
Conversation
Resolves #6902 |
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.
One comment below.
Do you have any thoughts on what the removal process for this config will look like? If we visit /app/kibana
kibana.defaultAppId
is used and defaultRoute won't trigger. Without a defaultAppId in this case we won't load a kibana app. I almost think it makes sense to keep both, and set the defaultRoute to app/kibana#discover to match.
src/core_plugins/kibana/index.js
Outdated
(settings, log) => { | ||
if (has(settings, 'defaultAppId')) { | ||
const newConfig = `server.defaultRoute: /app/kibana#/${get(settings, 'defaultAppId')}`; | ||
log(`Config key "defaultAppId" will be deprecated. Use the equivalent "${newConfig}"`); |
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.
thoughts on overriding defaultRoute here to /app/kibana#/${get(settings, 'defaultAppId')}
?
src/core_plugins/kibana/index.js
Outdated
(settings, log) => { | ||
if (has(settings, 'defaultAppId')) { | ||
const newConfig = `server.defaultRoute: /app/kibana#/${get(settings, 'defaultAppId')}`; | ||
log(`Config key "defaultAppId" will be deprecated. Use the equivalent "${newConfig}"`); |
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.
This should be "will be removed" right?
docs/setup/settings.asciidoc
Outdated
@@ -22,9 +22,9 @@ the `server.host` setting. When the value of this setting is `false`, Kibana use | |||
to this Kibana instance. | |||
`kibana.index:`:: *Default: ".kibana"* Kibana uses an index in Elasticsearch to store saved searches, visualizations and | |||
dashboards. Kibana creates a new index if the index doesn’t already exist. | |||
`kibana.defaultAppId:`:: *Default: "discover"* The default application to load. | |||
`kibana.defaultAppId:`:: *Default: "discover"* The default application to load. Use `server.defaultRoute`, as this will be deprecated in future versions. |
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.
This should be "will be removed" right?
Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
dcadfb8
to
12dd517
Compare
I updated the description to refer to removing this in 7.0 instead of 6.0, since we never got it deprecated. |
Closing this for now. |
We have long since replaced the
defaultAppId
withdefaultRoute
. This logs a deprecation warning so we can remove in 7.0.With
defaultAppId: discover