Implement querying mechanism #23

sandeepsajan0 · 2018-06-16T10:32:21Z

Now it loads only the API structure from the API doc like classendpoints, collectionendpoints, entrypoints and some of these properties and operations, not loads the data from the endpoint. Please suggest any changes.

xadahiya · 2018-06-16T17:08:46Z

@sandeepsajan0 The whole agent is supposed to work as a single thing. Why are you rewriting the code? You're supposed to refactor the code you wrote earlier to fit our needs not rewrite the entire thing.

sandeepsajan0 · 2018-06-17T21:17:29Z

Now, only the graph loads in Redis but after the first query data will be load in Redis in a way like for collection_endpoints, we will give endpoint_id to load_from_sever function and it will load the data for this particular endpoint to the Redis and all based on user's query. The code may be updated after the querying mechanism.

xadahiya · 2018-06-19T13:49:38Z

hydra-redis/classes_objects.py

+
+    def load_from_server(
+            self,
+            class_endpoint_id,


All you need to fetch data from server is url or the endpoint. Refactor the code and create simple functions

sandeepsajan0 · 2018-06-22T17:04:42Z

The latest commit I have done for the update of querying mechanism and I am still working on it. So, review it later when I will update it next time. Now, I am working on the modification of graph and also on querying mechanism. So, I'll update it soon.

Mec-iS · 2018-06-23T08:43:36Z

hydra-redis/querying_mechanism.py

+
+def final_file(url):
+    """ Open the given url and read and load the Json data."""
+    response = urllib.request.urlopen(url)


try-except block needed here. Every I/O call has to be safely written into an exception block, same for the JSON serialization below.

Mec-iS · 2018-06-23T08:45:28Z

hydra-redis/querying_mechanism.py

+        map_string = map(str.strip,string.split(','))
+        property_list = list(map_string)
+        check = property_list.pop()
+        property_list.append(check.replace("\x00",""))


How it happens that this character "\x00" ends up here?

"\x00" is null character and In C it works also as a string terminator character. Here is something related to this.

Mec-iS · 2018-06-23T08:48:04Z

hydra-redis/querying_mechanism.py

+        print (property_list)
+
+def endpoints_query(query):
+    r = redis.StrictRedis(host='localhost', port=6379, db=0)


You should factorized this at application run-time, and make the connection available through a proxy class or function like:

class RedisProxy: def __init__(self): self.connection = redis.StrictRedis... def get_connection(self): return self.connectoin

Mec-iS · 2018-06-23T08:53:19Z

See annotations in the code.

Other annotations:

I don't see any comment explaining the procedures
try to apply basic patterns like Proxy or Facade, I know it seems hard in the beginning but you should try to give a better design to the codebase since the beginning, refactoring has to speed up, you can write better scripts since the beginning if the code is more ordered
in general, work should go on faster, with more iterations

sandeepsajan0 · 2018-06-23T12:20:51Z

Thanks @Mec-iS , Actually I am still working on it and I will update it soon with all these things you have mentioned.

xadahiya · 2018-06-24T08:34:53Z

hydra-redis/hydra_graph.py

-if __name__ == "__main__":
+def main(new_url,api_doc):
    redis_con = redis.Redis(host='localhost', port=6379)
+    global redis_graph


Instead of creating a global variable for the graph you should pass the graph as a function argument whenever required.

Global variables should be used very carefully. You should avoid using them unless it's absolutely necessary.

Actually, we need to import redis_graph attribute in querying_mechanism. So, I am using this as a global variable.

xadahiya · 2018-06-24T08:37:09Z

hydra-redis/querying_mechanism.py

+        count = 2
+
+    for objects in reply:
+        count+=1


What is count? How does it work?
Add DOCStrings and proper comments

xadahiya · 2018-06-24T08:39:36Z

hydra-redis/querying_mechanism.py

+            global url
+            url =input("url>>>")
+            apidoc = final_file(url + "/vocab")
+            global api_doc


Again, avoid using global variables

xadahiya · 2018-06-24T08:42:32Z

hydra-redis/querying_mechanism.py

+                                  'apidoc','MATCH (p:classes)' 'RETURN p')
+        count = 0
+
+    if query == "collectionEndpoint":


Try to write small and separate functions for different types of queries. It'll make the code more readable and easier to maintain.

sandeepsajan0 · 2018-06-28T07:44:23Z

Querying mechanism is working now but I am still working on some of its issues like:

faceted index for every node properties.
error handling.
add more features.
Add new data in Redis memory #24
@Mec-iS I have a point that it needs to indexing for every node's properties? If yes please elaborate a bit for me. As I think about it, it can partially resolve the Add new data in Redis memory #24 because if we use faceted indexing then we can show the data to the user with help of indexing. So, we don't need to access the graph for data and user will not get the problem of multiple data. Am I right? Please explain more.

Mec-iS · 2018-06-29T11:59:27Z

Faceted indexes work because they index only few properties of an object. Indexing the full object means storing the object twice. For now we need only to index the unique id of an object, that's why I asked to work on a globally unique id for every entity.

Once we have unique ids, we can talk about synchronization and querying.

The idea is to store only predicates that relates objects ids. So:

unique_id -> some_prop -> unique_id

Again, that is why we need global ids in place.

xadahiya

@sandeepsajan0 You should use Docstrings as vaibhav is using here HTTP-APIs/hydrus#228

xadahiya · 2018-07-03T09:35:31Z

hydra-redis/classes_objects.py

+                endpoint][
+                "class"].supportedProperty:
+            if (endpoint != support_property.title and
+                    support_property.title not in self.class_endpoints):


Add comments explaining what you're doing here.

xadahiya · 2018-07-03T09:36:02Z

hydra-redis/classes_objects.py

+                    member[support_property.title] = str(
+                        new_file[support_property.title].replace(" ", ""))
+                else:
+                    no_endpoint_property = new_file[support_property.title]


Add comments to explain this as well.

xadahiya · 2018-07-03T09:41:04Z

hydra-redis/classes_objects.py

+                                endpoint_property]:
                            for nodes in self.redis_graph.nodes.values():
                                if endpoints == nodes.alias:
                                    self.addEdge(


What's the point of using break in line 182?

If we got the endpoint once in redis_graph nodes then no need to go ahead in the redis_graph. So, I am using the break for that.

xadahiya · 2018-07-03T09:45:21Z

hydra-redis/collections_endpoint.py

+                print("member alias and url+id",
+                      member_alias.capitalize(),
+                      base_url + member_id)
                new_url1 = new_url + "/" + member_id


Rename this and use proper variables.

xadahiya · 2018-07-03T09:46:42Z

hydra-redis/collections_endpoint.py

                            member[support_property.title] = str(
-                                new_file1[support_property.title].replace(" ", ""))
+                                new_file1[
+                                    support_property.title].replace(" ", ""))


Add comments to explain what you're doing and why.

xadahiya · 2018-07-03T15:43:53Z

hydra-redis/querying_mechanism.py

+
+    def get_classes_properties(self, query):
+        """
+        Show the given type of property of given Class endpoint.


Explain what query is with example. In general, you should use the format vaibhav is using for docstrings.
See this HTTP-APIs/hydrus#228

xadahiya · 2018-07-03T15:54:20Z

hydra-redis/querying_mechanism.py

+                faceted_list.append(self.faceted_key(key, value))
+            if len(query) > 0:
+                operation, query = query.split(" ", 1)
+                if operation == "or":


What if there are multiple key:value pair and not all have or. Like what it two conditions have and and the rest have or and similar situations.
You should handle such scenarios as well.

Yeah, there are some issues like this. I am working on it.

xadahiya · 2018-07-03T16:19:31Z

hydra-redis/querying_mechanism.py

+    url = input("url>>>")
+    handle_data = HandleData()
+    apidoc = handle_data.load_data(url + "/vocab")
+    while (1):


Use while(True) instead. Also, you should display what errors occurred in APIDoc loading failed.

Okay.
Yeah, we are displaying the error in HandleData (line 26-30).

xadahiya · 2018-07-03T16:20:25Z

hydra-redis/querying_mechanism.py

+    facades.initialize()
+    global check_list
+    check_list = []
+    while(1):


You should do something like this, it's much more elegant.

You can also create a help() to return the help strings.

xadahiya · 2018-07-03T16:21:52Z

hydra-redis/querying_mechanism.py

+    print("for collection properties:-",
+          "show <collection_endpoint> properties")
+    print("for classes properties:- show class<class_endpoint> properties")
+    print("for compare properties:-show <key> <value> and/or <key1> <value1>")


Clear this, use the method I've suggested above.

xadahiya · 2018-07-07T05:44:23Z

hydra-redis/test/querying_test.py

+
+PARENT_DIR = path.dirname(path.dirname(path.abspath(__file__)))
+sys.path.append(PARENT_DIR)
+import querying_mechanism


Use absolute imports everywhere not relative imports

I am trouble here, I have tried or read many things but I don't find any for that. Please help for that.

xadahiya · 2018-07-07T05:45:19Z

hydra-redis/test/querying_test.py

+        """Test for all the endpoints class + collection endpoints"""
+        query = "show endpoints"
+        data = query_facades.user_query(query)
+        if len(data) == 11:


You need to use assert statements in tests. Also, don't just compare the length you need to test data as well.

I think we don't need to test for all the endpoint because we have two different test for class endpoints and collection endpoints. So, These two can cover this endpoint test. And Yeah, I have change comparison of length to comparison of data. I'll push it early.

xadahiya · 2018-07-07T05:47:31Z

hydra-redis/test/querying_test.py

+            return False
+
+
+class TestQueryingMechanism(unittest.TestCase):


You should define tests here only. Why are you defining them outside?

xadahiya · 2018-07-07T05:48:08Z

hydra-redis/test/querying_test.py

+
+
+class TestQueryingMechanism(unittest.TestCase):
+    query_test = Tests()


Also, use teardown and setup methods for testing.

I can not figure out, what should I put in setUp and tearDown function? I think there are the different things in every test because of the different query in every test nothing is similar to put in setUp function.

xadahiya

You still haven't fixed all the things I pointed out earlier. Also, see the issues I've commented upon.

sandeepsajan0 · 2018-07-08T07:36:10Z

I think, only one major issue is remaining that user can input and and or both in one query.
I need some time for that. Can I open an issue for that and can discuss there if required?

xadahiya · 2018-07-09T03:22:18Z

hydra-redis/test/querying_test.py

+        self.assertEqual(data,check_data)


 if __name__ == "__main__":


Sandeep you're supposed to initialize the graph and everything in the setUp function. It is meant to do the same. Rest everything looks fine for now. Create an issue for the and or query part then we can merge this.

@xadahiya If we initialize the graph every time for every test with setup method then there will be data stored multiple times in Redis server(because Redis server is running independently to the files). And It'll provide duplicate data for which we already have an issue #24 ( I am working on it). And due to duplicate data test cases will fail.

xadahiya · 2018-07-09T04:14:55Z

hydra_redis/test/querying_test.py

+        self.assertEqual(data,check_data)
+
+
+if __name__ == "__main__":


@sandeepsajan0 You need to include all this in the setUp function, rest everything looks fine for now. Fix this then we can merge your PR.

Just call unittest.main() here

add lazyload graph

2b71638

sandeepsajan0 added 3 commits June 18, 2018 02:14

lazyload graph

d109f5c

update lazyload graph

950b24b

follow pep8 style

646ae29

xadahiya requested changes Jun 19, 2018

View reviewed changes

update of querying mechanism

f713429

Mec-iS reviewed Jun 23, 2018

View reviewed changes

Mec-iS mentioned this pull request Jun 23, 2018

Added semantic refs to classes HTTP-APIs/hydrus#210

Closed

xadahiya reviewed Jun 24, 2018

View reviewed changes

xadahiya requested changes Jun 24, 2018

View reviewed changes

update coding style and use try-except

fd2e1f4

sandeepsajan0 force-pushed the sandeeppatch1 branch from 5b2a7c4 to fd2e1f4 Compare June 26, 2018 19:43

sandeepsajan0 added 2 commits June 27, 2018 12:18

follow style pep8

b2c25af

add faceted_indexing to properties

ce32264

sandeepsajan0 force-pushed the sandeeppatch1 branch from d573aac to ce32264 Compare June 28, 2018 07:33

xadahiya requested changes Jul 3, 2018

View reviewed changes

xadahiya mentioned this pull request Jul 6, 2018

Implement a querying mechanism for loaded Graph #20

Closed

sandeepsajan0 changed the title ~~add lazyload graph~~ Implement querying mechanism Jul 6, 2018

update querying mechanism with docstrings and comments

a968bb4

sandeepsajan0 added 2 commits July 6, 2018 15:38

add tests for querying process

373a8b6

update coding style

6dedee4

xadahiya reviewed Jul 7, 2018

View reviewed changes

xadahiya requested changes Jul 7, 2018

View reviewed changes

sandeepsajan0 added 3 commits July 8, 2018 01:24

remove globalvariable, update querying_test

ffde51e

rename directory

a18106b

remove previous files

0f189ee

xadahiya reviewed Jul 9, 2018

View reviewed changes

xadahiya requested changes Jul 9, 2018

View reviewed changes

add setup and teardown method

24e2b1a

sandeepsajan0 force-pushed the sandeeppatch1 branch from b604c37 to 24e2b1a Compare July 9, 2018 04:28

Merge branch 'develop' into sandeeppatch1

b134f0c

xadahiya approved these changes Jul 9, 2018

View reviewed changes

xadahiya merged commit 5b9fa10 into HTTP-APIs:develop Jul 9, 2018

sandeepsajan0 deleted the sandeeppatch1 branch February 6, 2019 06:52



		class TestQueryingMechanism(unittest.TestCase):
		query_test = Tests()

Implement querying mechanism #23

Implement querying mechanism #23

Uh oh!

Conversation

sandeepsajan0 commented Jun 16, 2018

Uh oh!

xadahiya commented Jun 16, 2018

Uh oh!

sandeepsajan0 commented Jun 17, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sandeepsajan0 commented Jun 22, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sandeepsajan0 Jun 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mec-iS commented Jun 23, 2018

Uh oh!

sandeepsajan0 commented Jun 23, 2018

Uh oh!

xadahiya Jun 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sandeepsajan0 commented Jun 28, 2018

Uh oh!

Mec-iS commented Jun 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xadahiya left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sandeepsajan0 Jun 23, 2018 •

edited

Loading

xadahiya Jun 24, 2018 •

edited

Loading

Mec-iS commented Jun 29, 2018 •

edited

Loading