Http service #22

mudit1403 · 2018-07-25T08:11:53Z

Added Http Service.

Tushar-Naik · 2018-07-26T10:38:59Z

src/main/java/com/flipkart/ranger/finder/AbstractServiceRegistry.java

+import java.util.List;
+import java.util.concurrent.*;
+
+public abstract class AbstractServiceRegistry<T> implements ServiceRegistry<T> {


remove this class

Tushar-Naik · 2018-07-26T10:39:26Z

src/main/java/com/flipkart/ranger/finder/AbstractServiceRegistryUpdater.java

+        }
+    }
+
+    protected abstract List<ServiceNode<T>> getServiceNodes();


rename this method

Tushar-Naik · 2018-07-26T10:42:10Z

src/main/java/com/flipkart/ranger/finder/HttpSourceConfig.java

+import org.slf4j.LoggerFactory;
+
+public class HttpSourceConfig extends SourceConfig{
+    private static final Logger logger = LoggerFactory.getLogger(CuratorSourceConfig.class);


wrong class, should be HttpSourceConfig

Tushar-Naik · 2018-07-26T10:43:27Z

src/main/java/com/flipkart/ranger/finder/CuratorSourceConfig.java


-public class Service {
    private CuratorFramework curatorFramework;


remove curatorFramework from the config

Tushar-Naik · 2018-07-26T10:43:51Z

src/main/java/com/flipkart/ranger/finder/ServiceVisitor.java

@@ -0,0 +1,7 @@
+package com.flipkart.ranger.finder;
+
+public interface ServiceVisitor<T> {


Rename this class, to maybe SourceConfigVisitor

Tushar-Naik · 2018-07-26T10:44:36Z

src/main/java/com/flipkart/ranger/finder/sharded/MapBasedServiceRegistry.java

@@ -27,12 +27,12 @@
 import java.util.List;
 import java.util.concurrent.atomic.AtomicReference;

-public class MapBasedServiceRegistry<T> extends AbstractZookeeperServiceRegistry<T> {
+public class MapBasedServiceRegistry<T> extends AbstractServiceRegistry<T> {


would be implementing ServiceRegistry directly

Tushar-Naik · 2018-07-26T10:46:34Z

src/main/java/com/flipkart/ranger/finder/BaseServiceFinderBuilder.java

    private String namespace;
    private String serviceName;
    private CuratorFramework curatorFramework;
+    private String host;


Keep only curatorFramework and curatorSourceConfig and httpSourceConfig
and simplify the builders to withCuratorSourceConfig() and withHttpSourceConfig()

Tushar-Naik · 2018-07-26T10:49:41Z

src/main/java/com/flipkart/ranger/finder/CuratorServiceRegistryUpdater.java

+
+    @Override
+    public void start() throws Exception {
+        curatorFramework = config.getCuratorFramework();


start the curator framework here

Tushar-Naik · 2018-07-26T10:50:28Z

src/main/java/com/flipkart/ranger/finder/BaseServiceFinderBuilder.java

+                .namespace(namespace)
+                .connectString(connectionString)
+                .retryPolicy(new ExponentialBackoffRetry(1000, 100)).build();
+        curatorFramework.start();


this will not be required here

…r, removed CuratorFramework from config and other minor changes

Tushar-Naik · 2018-07-26T12:26:30Z

src/main/java/com/flipkart/ranger/finder/BaseServiceFinderBuilder.java

+            HttpServiceRegistryUpdater<T> registryUpdater = new HttpServiceRegistryUpdater<T>(httpConfig, deserializer);
+            return buildFinder(httpConfig, registryUpdater, deserializer, shardSelector, nodeSelector, healthcheckRefreshTimeMillis);
+        }
+        if (null != curatorConfig) {


if (curatorFramework !=null) use the curatorFramework instead of building one

Tushar-Naik · 2018-07-26T12:30:51Z

src/main/java/com/flipkart/ranger/finder/BaseServiceFinderBuilder.java

+            return buildFinder(curatorConfig, registryUpdater, deserializer, shardSelector, nodeSelector, healthcheckRefreshTimeMillis);
+        }
+        //TODO: what should be the default case?
+        return buildFinder(null, null, deserializer, shardSelector, nodeSelector, healthcheckRefreshTimeMillis);


make the code look like before

if( null == curatorFramework) { Preconditions.checkNotNull(connectionString); curatorFramework = CuratorFrameworkFactory.builder() .namespace(namespace) .connectString(connectionString) .retryPolicy(new ExponentialBackoffRetry(1000, 100)).build(); curatorFramework.start(); } CuratorServiceRegistryUpdater<T> registryUpdater = new CuratorServiceRegistryUpdater<T>(curatorConfig, deserializer, curatorFramework); return buildFinder(curatorConfig, registryUpdater, deserializer, shardSelector, nodeSelector, healthcheckRefreshTimeMillis);

Tushar-Naik

Add a test case for HttpSourceConfig based serviceFinder. Use wiremock to mock the response of the url

Tushar-Naik · 2018-07-26T14:25:34Z

src/main/java/com/flipkart/ranger/finder/HttpServiceRegistryUpdater.java

+        httpclient = HttpClients.createDefault();
+
+        serviceRegistry.nodes(getHealthyServiceNodes());
+        logger.info("Started polling zookeeper for changes");


change log line

Tushar-Naik · 2018-07-26T14:26:50Z

src/main/java/com/flipkart/ranger/finder/HttpServiceRegistryUpdater.java

+            final String host = config.getHost();
+            final Integer port = config.getPort();
+            final String path = config.getPath();
+            final URI uri = new URIBuilder()


URI could be created in the constructor once

Tushar-Naik · 2018-07-26T14:30:15Z

src/main/java/com/flipkart/ranger/finder/AbstractServiceRegistryUpdater.java

+        }
+    }
+
+    //TODO: rename this method


remove todo, add documentation to explain what the method is intended to do

Tushar-Naik · 2018-07-26T14:32:03Z

src/main/java/com/flipkart/ranger/finder/BaseServiceFinderBuilder.java

-                    .connectString(connectionString)
-                    .retryPolicy(new ExponentialBackoffRetry(1000, 100)).build();
-            curatorFramework.start();
-        }
        if( 0 == healthcheckRefreshTimeMillis) {


Preconditions.checkNotNull(serviceName); required

…emoved errors

pom.xml

Tushar-Naik · 2018-08-01T05:24:25Z

src/main/java/com/flipkart/ranger/finder/BaseServiceFinderBuilder.java

@@ -1,12 +1,12 @@
 /**
 * Copyright 2015 Flipkart Internet Pvt. Ltd.
- *
+ * <p>


para not required

Tushar-Naik · 2018-08-01T05:31:05Z

src/test/java/com/flipkart/ranger/httpservicefinder/UnshardedTimerTest.java

+                            objectMapper.readValue(data,
+                                    new TypeReference<List<ServiceNode<EnvNodeData>>>() {
+                                    });
+                    if (integer.incrementAndGet() > 2) {


Add a comment to signify what we are doing here

santanusinha · 2018-08-10T04:15:56Z

pom.xml

@@ -114,8 +120,8 @@
                <artifactId>maven-compiler-plugin</artifactId>
                <version>3.1</version>
                <configuration>
-                    <source>1.7</source>
-                    <target>1.7</target>
+                    <source>8</source>


santanusinha · 2018-08-10T04:28:09Z

src/main/java/com/flipkart/ranger/finder/HttpServiceRegistryUpdater.java

+
+            try (CloseableHttpResponse response = httpclient.execute(httpget)) {
+                if (response.getStatusLine().getStatusCode() != HttpStatus.SC_OK) {
+                    logger.error("Error in Http get");


We need to catch the response and clearly print it out in logs if this happens including the status.

santanusinha · 2018-08-10T04:28:58Z

src/main/java/com/flipkart/ranger/finder/HttpServiceRegistryUpdater.java

+
+                List<ServiceNode<T>> serviceNodes = config.getListDeserializer().deserialize(data);
+                return serviceNodes.stream()
+                        .filter(node -> (node.getHealthcheckStatus() == HealthcheckStatus.healthy && node.getLastUpdatedTimeStamp() > healthcheckZombieCheckThresholdTime))


Break this line

santanusinha · 2018-08-10T04:31:38Z

src/main/java/com/flipkart/ranger/finder/HttpServiceRegistryUpdater.java

+import java.util.List;
+import java.util.stream.Collectors;
+
+public class HttpServiceRegistryUpdater<T> extends AbstractServiceRegistryUpdater<T> {


Ok, so there are multiple assumption in this code:

Rename the ListDeserializer interface to HttpResponseDecoder interface

It assumes the return status code will be 200. Might not be.

It assumes, the HTTP verb will be get. It can be post for whatever reason.

santanusinha · 2018-08-10T04:32:06Z

src/main/java/com/flipkart/ranger/finder/HttpServiceRegistryUpdater.java

+                    return null;
+                }
+
+                List<ServiceNode<T>> serviceNodes = config.getListDeserializer().deserialize(data);


Add a transformer in the middle. Response might not be directly consumable.

santanusinha · 2018-08-10T04:56:03Z

src/main/java/com/flipkart/ranger/finder/SourceConfig.java

+    public ServiceType getServiceType() {
+        return serviceType;
+    }
+}


Add visitor interface here.

santanusinha · 2018-08-10T05:01:38Z

src/main/java/com/flipkart/ranger/finder/HttpServiceRegistryUpdater.java

+public class HttpServiceRegistryUpdater<T> extends AbstractServiceRegistryUpdater<T> {
+    private static final Logger logger = LoggerFactory.getLogger(HttpServiceRegistryUpdater.class);
+
+    private HttpSourceConfig config;


Pass in type param to HttpSourceConfig

santanusinha · 2018-08-10T05:07:25Z

src/main/java/com/flipkart/ranger/finder/CuratorSourceConfig.java

+public class CuratorSourceConfig<T> extends SourceConfig{
+    private String connectionString;
+    private String namespace;
+    private Deserializer<T> deserializer;


Since this is only for handling ZK Specific data, we should rename it ZookeeprNodeDataDecoder

santanusinha · 2018-08-10T05:07:43Z

src/main/java/com/flipkart/ranger/finder/CuratorSourceConfig.java

+
+import com.flipkart.ranger.model.Deserializer;
+
+public class CuratorSourceConfig<T> extends SourceConfig{


Rename this to ZookeeperSourceConfig

santanusinha · 2018-08-10T05:13:38Z

src/main/java/com/flipkart/ranger/model/ListDeserializer.java

+
+import java.util.List;
+
+public interface ListDeserializer<T> {


HttpResponseDecoder

…nd https, renaming and other minor changes

…erics

Mudit G added 5 commits July 24, 2018 16:25

initial try on adding http communication

2b74f82

gave better name for SchedulerServiceRegistry class

1f8c065

moved httpclient creation to start of HttpServiceRegistryUpdater

998dbe3

restructuring by moving few functionalities from serviceRegistry

b1bc7f1

added zookeeper cluster connection to CuratorServiceRegistryUpdater

e018009

Tushar-Naik assigned mudit1403 Jul 26, 2018

Tushar-Naik reviewed Jul 26, 2018

View reviewed changes

removed AbstractServiceRegistry, restructured BaseServiceFinderBuilde…

3eea49a

…r, removed CuratorFramework from config and other minor changes

Tushar-Naik reviewed Jul 26, 2018

View reviewed changes

added withCuratorFramework and restructured test as per new builder

7f55e47

Tushar-Naik reviewed Jul 26, 2018

View reviewed changes

Mudit G added 3 commits July 27, 2018 16:19

restructured code, test cases restructured as per new structure and r…

d836f1f

…emoved errors

added http based communication tests

9a8ad22

added license

45902a0

Tushar-Naik reviewed Aug 1, 2018

View reviewed changes

Mudit G added 6 commits August 1, 2018 11:20

added tests comment, cleanup and added test scope

dc12a40

removed error in ServiceHealthAggregatorTest

a6d049a

minor change in ServiceHealthAggregatorTest

bc77425

removing error in ServiceProviderExtCuratorTest

5dab01a

corrected ServiceHealthAggregatorTest

2c90b0a

cleanup

ae6a6f7

santanusinha requested changes Aug 10, 2018

View reviewed changes

Mudit G added 2 commits August 13, 2018 11:35

design changes to accept Source Config, added support for Http Post a…

023d9f5

…nd https, renaming and other minor changes

allow for suppressing host check in https, refactoring and proper gen…

07940bc

…erics

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Http service #22

Http service #22

mudit1403 commented Jul 25, 2018

Tushar-Naik Jul 26, 2018

Tushar-Naik Jul 26, 2018

Tushar-Naik Jul 26, 2018

Tushar-Naik Jul 26, 2018

Tushar-Naik Jul 26, 2018

Tushar-Naik Jul 26, 2018

Tushar-Naik Jul 26, 2018

Tushar-Naik Jul 26, 2018

Tushar-Naik Jul 26, 2018

Tushar-Naik Jul 26, 2018

Tushar-Naik Jul 26, 2018

Tushar-Naik left a comment

Tushar-Naik Jul 26, 2018

Tushar-Naik Jul 26, 2018

Tushar-Naik Jul 26, 2018

Tushar-Naik Jul 26, 2018 •

edited

Loading

Tushar-Naik Aug 1, 2018

Tushar-Naik Aug 1, 2018

santanusinha Aug 10, 2018

santanusinha Aug 10, 2018

santanusinha Aug 10, 2018

santanusinha Aug 10, 2018

santanusinha Aug 10, 2018

santanusinha Aug 10, 2018

santanusinha Aug 10, 2018

santanusinha Aug 10, 2018

santanusinha Aug 10, 2018

santanusinha Aug 10, 2018


		public class Service {
		private CuratorFramework curatorFramework;

		@@ -0,0 +1,7 @@
		package com.flipkart.ranger.finder;

		public interface ServiceVisitor<T> {


		import com.flipkart.ranger.model.Deserializer;

		public class CuratorSourceConfig<T> extends SourceConfig{


		import java.util.List;

		public interface ListDeserializer<T> {

Http service #22

Are you sure you want to change the base?

Http service #22

Conversation

mudit1403 commented Jul 25, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Tushar-Naik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Tushar-Naik Jul 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Tushar-Naik Jul 26, 2018 •

edited

Loading