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

Re-set default index pattern in navigateToApp #8125

Merged
merged 9 commits into from
Aug 30, 2016

Conversation

LeeDr
Copy link

@LeeDr LeeDr commented Aug 30, 2016

Fixes #7496. Several Jenkins selenium test runs have failed again recently when there is a logstash-* index pattern but it is not set as the default even though the test successfully called the method to set it;

esClient.deleteAndUpdateConfigDoc({'dateFormat:tz':'UTC', 'defaultIndex':'logstash-*'})

This could be caused by #7055 where Kibana writes a change to the config doc after the automation set the defaultIndex, thus clobbering it.

This PR works around that by checking if we're navigating to discover, visualize, or dashboard, and don't have a default index pattern set. In that case it logs a WARNING and sets it to logstash-*.

What if we write a test that needs a different default index pattern than logstash-*? We'll have to fix this better.

The easiest way to test this is to change this line in the very first test;
https://github.com/elastic/kibana/blob/master/test/functional/apps/discover/_discover.js#L19

from;

return esClient.deleteAndUpdateConfigDoc({'dateFormat:tz':'UTC', 'defaultIndex':'logstash-*'})

to;

return esClient.deleteAndUpdateConfigDoc({'dateFormat:tz':'UTC'})

This would cause the code below to log the warning and set the default index pattern (and then continue to pass tests).

        return esClient.getDefaultIndex()
        .then(function (defaultIndex) {
          if (appName === 'discover' || appName === 'visualize' || appName === 'dashboard') {
            if (!defaultIndex) {
              // https://github.com/elastic/kibana/issues/7496
              // Even though most tests are using esClient to set the default index, sometimes Kibana clobbers
              // that change.  If we got here, fix it.
              self.debug(' >>>>>>>> WARNING Navigating to [' + appName + '] with defaultIndex=' + defaultIndex);
              self.debug(' >>>>>>>> Setting defaultIndex to "logstash-*""');
              esClient.updateConfigDoc({'dateFormat:tz':'UTC', 'defaultIndex':'logstash-*'});
            }
          }

// that change. If we got here, fix it.
self.debug(' >>>>>>>> WARNING Navigating to [' + appName + '] with defaultIndex=' + defaultIndex);
self.debug(' >>>>>>>> Setting defaultIndex to "logstash-*""');
esClient.updateConfigDoc({'dateFormat:tz':'UTC', 'defaultIndex':'logstash-*'});
Copy link
Contributor

Choose a reason for hiding this comment

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

Does updateConfigDoc return a promise? If so, I think we should return that promise and then do the self.remote.get() call in a followup .then() block.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, I think you're right about chaining the returned promise. I'll make that change.

@epixa
Copy link
Contributor

epixa commented Aug 30, 2016

I pushed this up in another PR to make it run the selenium tests, and it failed: https://kibana-ci.elastic.co/job/elastic+kibana+pull-request+multijob-intake/711/consoleFull

@LeeDr
Copy link
Author

LeeDr commented Aug 30, 2016

The failure in https://kibana-ci.elastic.co/job/elastic+kibana+pull-request+multijob-intake/711/consoleFull is caused by the side nav expand/collapse change which is on another issue.

spalger and others added 4 commits August 30, 2016 11:11
Several tests have been failing recently with "Error: Unexpected request: POST /api/kibana/settings/shortDots:enable, No more request expected" which seems to be caused by the field chooser tests calling `config.set('shortDots:enable', origValue)` in the test cleanup task. Rather than investigate it further I avoided the need to call `config.set()` by stubbing the `config.get()` method.
Changed required field data pane width and increased window width for…
@epixa
Copy link
Contributor

epixa commented Aug 30, 2016

Can you rebase this since we merged that other PR? I'd love to test it together.

@LeeDr
Copy link
Author

LeeDr commented Aug 30, 2016

jenkins, test this

@epixa epixa merged commit edf1ee8 into elastic:master Aug 30, 2016
airow pushed a commit to airow/kibana that referenced this pull request Feb 16, 2017
Re-set default index pattern in navigateToApp

Former-commit-id: edf1ee8
@LeeDr LeeDr deleted the defaultIndexPattern7496 branch May 23, 2017 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

logstash-* index pattern is created but is not the default
2 participants