Skip to content
This repository has been archived by the owner on Jan 10, 2019. It is now read-only.

multi-dc-seed-sync #120

Merged
merged 1 commit into from
Jul 17, 2015
Merged

multi-dc-seed-sync #120

merged 1 commit into from
Jul 17, 2015

Conversation

dmitrypekar
Copy link
Contributor

Hey, Ben.

Just pushed final changes to support dc syncing.
For now I am going to run some more tests (99% that they would pass).
During that time you can review the code and post your comments.

@@ -36,6 +40,17 @@ public static String get(@NotNull final String key) {
}

@NotNull
public static Map<String, String> optionsStart(@NotNull final String prefix, final boolean trimPrefix) {
Copy link
Contributor

Choose a reason for hiding this comment

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

please rename method to filterStartsWith(@NotNull final String prefix, final boolean trimPrefix).

The name option on line 38 was used to represent an optional value for a given key, but in this scenario we're already returning a 0-many data structure we won't need option in the name. In actuality we're really filtering all of the environment down to those entries whose key begins with prefix.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@BenWhitehead
Copy link
Contributor

@dmitrypekar See note about ObjectMapper.

Once that's fixed, please rebase squash this whole PR in prep for merge.

@dmitrypekar
Copy link
Contributor Author

Squashed to a single commit. Could be merged now.

BenWhitehead added a commit that referenced this pull request Jul 17, 2015
@BenWhitehead BenWhitehead merged commit a6a94fb into mesosphere-backup:master Jul 17, 2015
@BenWhitehead
Copy link
Contributor

Merged, thanks @dmitrypekar.

@BenWhitehead
Copy link
Contributor

I've got a preliminary change to the package/config definition for the universe that I'll and can get a pr of soon so that we can provide this new config for dcos installs.

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.

2 participants