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.

Sign up for GitHub

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

Jump to bottom

Closes #2377 - Migrate config from GA and Beyond #2745

Merged
merged 19 commits into from
Jan 30, 2015
Merged

Closes #2377 - Migrate config from GA and Beyond #2745

merged 19 commits into from
Jan 30, 2015

Conversation

simianhacker
Copy link
Member

@simianhacker
Closes elastic#2377 - Migrate config from > GA
2b98020 
- Closes elastic#2377
- Added testing framework for server side tests
- Fixed jshint
- Added config migration
- Added framework for server initialization
@simianhacker
Refactoring the else statements out of isUpgradeable
253eeab
@spalger spalger modified the milestone: 4.0.0-RC1 Jan 28, 2015
@simianhacker
Merge branch 'master' of github.com:elasticsearch/kibana into feature…
22bfd9e 
…/config-migration

Conflicts:
	package.json
	src/server/routes/proxy.js
	tasks/test.js
@w33ble w33ble assigned w33ble and simianhacker and unassigned spalger, w33ble and simianhacker Jan 28, 2015
@w33ble w33ble added discuss and removed discuss labels Jan 28, 2015
w33ble
w33ble reviewed Jan 29, 2015
View reviewed changes
src/server/lib/migrateConfig.js
if (config.kibana.elasticsearch_username && config.kibana.elasticsearch_password) {
uri.auth = util.format('%s:%s', config.kibana.elasticsearch_username, config.kibana.elasticsearch_password);
}
console.log(url.format(uri));
Copy link
Contributor

Choose a reason for hiding this comment

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

left in by accident?

@w33ble
Copy link
Contributor

w33ble commented Jan 29, 2015

You've got a lot of ../../../../ style paths in here - I've used requireform in the past to help with that - cleans things up a bit. Especially in the case of tests, it would probably be useful to have a path helper or config file if you don't want to use requirefrom.

@w33ble
Copy link
Contributor

w33ble commented Jan 29, 2015

If the config (and thus the doc) does not have an _id field, as is the case with everyone running a beta right now, isUpgradeable throws TypeError: Cannot call method 'match' of undefined. I'd like to see a test for that case, and ideally some simple tests around upgradeConfig.js too.

Also, since in dev our _id version is @@version, I'd like to see a test case invalid version numbers too - currently it throws TypeError: Invalid Version: @@version.

@w33ble w33ble assigned simianhacker and unassigned w33ble Jan 29, 2015
@w33ble w33ble removed the review label Jan 29, 2015
@simianhacker simianhacker assigned w33ble and unassigned simianhacker Jan 29, 2015
@w33ble
Copy link
Contributor

w33ble commented Jan 29, 2015

Actually, I guess I'm wrong. Old betas still use the version as the _id, so this should be fine.

screenshot 2015-01-29 11 11 24

In development, we're using @@version, which may cause some issues in the future (like configs not upgrading when we bump version), but that's outside the scope of this PR.

@w33ble w33ble removed the review label Jan 29, 2015
@w33ble
Copy link
Contributor

w33ble commented Jan 30, 2015

let's get this merged 🐲

w33ble added a commit that referenced this pull request Jan 30, 2015
@w33ble
Merge pull request #2745 from simianhacker/feature/config-migration
0ae60f7 
Closes #2377 - Migrate config from GA and Beyond
@w33ble w33ble merged commit 0ae60f7 into elastic:master Jan 30, 2015
@simianhacker simianhacker deleted the feature/config-migration branch March 16, 2017 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@w33ble w33ble

Labels
review
Projects
None yet
Milestone
4.0.0-RC1
Development

Successfully merging this pull request may close these issues.

Migrate config when upgrading
4 participants
@simianhacker @w33ble @spalger @lukasolson