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

Removing ambry-coordinator #449

Merged
merged 2 commits into from
Sep 16, 2016
Merged

Conversation

vgkholla
Copy link
Contributor

@vgkholla vgkholla commented Sep 15, 2016

This change removes all usages of Coordinator and removes Coordinator code

Built and formatted.

Primary reviewers: @pnarayanan, @cgtz
Expected time to review: 20-30 mins

Addresses #432.

This change removes all usages of Coordinator and removes Coordinator code
Copy link
Contributor Author

@vgkholla vgkholla left a comment

Choose a reason for hiding this comment

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

Guidance comments.

}
}
};
TestFuture<GetBlobResult> testFuture =
Copy link
Contributor Author

Choose a reason for hiding this comment

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

only formatting changes.

@@ -1,170 +0,0 @@
/**
Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed this because it is unused and migrating it to the Router was unnecessary right now. There is already an issue (#383) to write a more comprehensive migrator service. I can like this PR to the issue in case someone wants to see what was available.

Copy link
Contributor

@pnarayanan pnarayanan left a comment

Choose a reason for hiding this comment

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

LGTM. I assume everything in the ambry-coordinator under git has been removed so the directory itself gets removed automatically (locally I see a few files but those are not under git).

@@ -271,7 +267,6 @@ private static PutResponse getPutResponseFromStream(ConnectedChannel blockingCha

public static void main(String args[]) {
FileWriter writer = null;
AmbryCoordinator coordinator = null;
Copy link
Contributor

Choose a reason for hiding this comment

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

This was just lying in here? The coordinator had no purpose in this tool?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup.

Copy link
Contributor

@cgtz cgtz left a comment

Choose a reason for hiding this comment

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

LGTM, make sure to run the server tests on linux if you haven't.

@vgkholla
Copy link
Contributor Author

Built on mac and linux.

@cgtz cgtz merged commit 903eab5 into linkedin:master Sep 16, 2016
@cgtz
Copy link
Contributor

cgtz commented Sep 16, 2016

merged

@vgkholla vgkholla deleted the coordinatorRemove branch September 16, 2016 04:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants