Skip to content
This repository has been archived by the owner on Nov 9, 2022. It is now read-only.

Bootstrap and wipe granularity and #538 Fix #567

Merged
merged 14 commits into from
Mar 4, 2016

Conversation

RobertSzkutak
Copy link
Contributor

The purpose of this pull request is to allow users to specify which parts of their applications to bootstrap using the bootstrap command.

Examples:

./ml local bootstrap --apply-changes=indexes,users,roles
(will only bootstrap the indexes, users, and roles)

./ml local wipe --apply-changes=databases,forests,appservers
(will only wipe databases, forests, and app servers)

This pull request accomplishes several things to make this possible:

  • Index configuration is now it's own function in setup.xqy . It was split out of database configuration.
  • setup:do-setup changed to respect the additional bootstrap options
  • server_config.rb bootstrap changed to handle the --apply-changes option and pass it on to setup:do-setup

I haven't tested everything and would appreciate a second set of eyes to test as well as feedback on any ways that the code could be improved.

Additionally, this branch fixes this bug with CORB: #538

@@ -949,7 +955,7 @@ def load_data(dir, options = {})
batch = @environment != "local" && batch_override.blank? || batch_override.to_b

options[:batch_commit] = batch
options[:permissions] = permissions(@properties['ml.app-role'], Roxy::ContentCapability::ER) unless options[:permissions]
options[:permissions] = permissions(@properties['ml.app-role'], Roxy::ContentCapability::ERU) unless options[:permissions]
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like there are a few redundant changes in this PR. Did you start off with upstream/dev, and did you fetch and rebase against that as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I did an upstream fetch from dev and then rebased against that. After that I began to make changes. Im dont think my other pull request for dev with this fix was accepted which may be why it was still lingering in my tree?

@grtjn
Copy link
Contributor

grtjn commented Feb 3, 2016

The change looks relatively simple. Can we do the same trick with wipe? That would be useful for testing this too..

@RobertSzkutak
Copy link
Contributor Author

Great idea! I'll build that in along with your suggested cleanup for checking the options.

@RobertSzkutak
Copy link
Contributor Author

I added in three more commits which make the changes you recommended as well as add identical functionality to wipe.

@RobertSzkutak RobertSzkutak changed the title Bootstrap granularity Bootstrap and wipe granularity Feb 3, 2016
@joemfb
Copy link
Contributor

joemfb commented Feb 4, 2016

Here's what I usually do to fix up these kinds of rebase issues:

git checkout dev
git pull upstream dev
git checkout rob
git rebase dev
git push origin rob -f

If that doesn't correctly isolate your commits, you may need to create a new branch and cherry-pick your commits specifically.

@RobertSzkutak
Copy link
Contributor Author

No luck, unfortunately :\

$ git checkout dev
Switched to branch 'dev'
$ git pull upstream dev
From https://github.com/marklogic/roxy
 * branch            dev        -> FETCH_HEAD
Already up-to-date.
$ git checkout rob
Switched to branch 'rob'
$ git rebase dev
Current branch rob is up to date.

Any other ideas?

I can take a look tonight at carefully constructing a cleaner branch.

@joemfb
Copy link
Contributor

joemfb commented Feb 4, 2016

Yeah, it looks like you'll need to. There's some good answers here: https://stackoverflow.com/questions/1994463/how-to-cherry-pick-a-range-of-commits-and-merge-into-another-branch.

I favor the three argument form of git rebase --onto; there's some good details here: https://blog.pivotal.io/labs/labs/git-rebase-onto.

@grtjn
Copy link
Contributor

grtjn commented Feb 4, 2016

I sometimes get unexpected commits appearing after squashing commits with git rebase -i. I usually get rid of them by doing another git rebase (without -i)

@RobertSzkutak
Copy link
Contributor Author

All fixed now?

I made one additional change to fix a merge conflict with this branch and the branch merged in yesterday (the #514 fix was not included in the branch merged in yesterday)

Full commit history for the branch seems clean: https://github.com/RobertSzkutak/roxy/commits/rob

@joemfb
Copy link
Contributor

joemfb commented Feb 8, 2016

I'm still seeing some older commits (like 4a276a from rloupre)...

You want to create a new branch from the latest marklogic/dev branch, and then populate with only the commits for this PR. You could accomplish that be editing your rob branch in place, or maybe just by creating a new branch from marklogic/dev and individually rebasing/cherry-picking your relevant commits from rob.

@RobertSzkutak
Copy link
Contributor Author

Third try is the charm?

Thank you for patience with me. I really appreciate your help.

@RobertSzkutak
Copy link
Contributor Author

Bump. Any chance of merging this in soon? One of our largest clients is using this pretty heavily right now and have been asking about when/if my changes will be "officially" supported.

@dmcassel
Copy link
Collaborator

Looks good to me. @grtjn have you already tested this?

@joemfb
Copy link
Contributor

joemfb commented Feb 22, 2016

FYI, the bulk of the changes in default.properties are line endings (you can ignore whitespace in diffs by adding ?w=0 to this PR URL).

@dmcassel
Copy link
Collaborator

Just ran a test.

$ ./ml local bootstrap
$ ./ml local wipe --apply-changes=roles

This successfully removes roxy-role, without removing roxy-user.

$ ./ml local bootstrap --apply-changes=roles

This restores roxy-role, but does not re-associate roxy-user with it. I think those two commands back-to-back should result in the same state as a full bootstrap.

@dmcassel
Copy link
Collaborator

Should this work?

$ ./ml local wipe --apply-changes=forests

I get

ERROR: ADMIN-DATABASEFORESTATTACHED: (err:FOER0000) Forest 17077807573084922939 is attached to a database

I think "wipe forests" should detach and remove and "bootstrap forests" should create and attach.

@dmcassel
Copy link
Collaborator

$ ./ml bootstrap -h

and

$ ./ml wipe -h 

should show the --apply-changes option, complete with what options are available. That's a pretty simple change in deploy/Help.rb.

@dmcassel
Copy link
Collaborator

@RobertSzkutak, thanks for persisting with this, looks like a handy capability.

@RobertSzkutak
Copy link
Contributor Author

Thanks for testing! We've been mostly focused on roles, users, and index usage at my client and I hadn't spotted these yet. I'll continue to improve this branch and have some new commits soon.

Josh Meekhof and others added 2 commits February 25, 2016 15:54
@RobertSzkutak
Copy link
Contributor Author

Merged in a pull request to my branch from @jmeekhof which fixes the user and role assignment issue pointed out by @dmcassel . @jmeekhof and I worked on and tested this out together and it looks good.

Josh Meekhof and others added 2 commits February 25, 2016 16:26
@RobertSzkutak
Copy link
Contributor Author

Added help documentation courtesy of @jmeekhof

@RobertSzkutak
Copy link
Contributor Author

Pushed in a fix for this issue: #538

My client has been using this branch heavily and needed the fix immediately.

@RobertSzkutak RobertSzkutak changed the title Bootstrap and wipe granularity Bootstrap and wipe granularity and #538 Fix Mar 3, 2016
@dmcassel
Copy link
Collaborator

dmcassel commented Mar 4, 2016

This passes my tests now. Thanks for submitting, @RobertSzkutak

dmcassel added a commit that referenced this pull request Mar 4, 2016
Bootstrap and wipe granularity and #538 Fix
@dmcassel dmcassel merged commit e7f7385 into marklogic-community:dev Mar 4, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants