Inter adapter communication #36

ANeaves · 2019-03-11T13:45:13Z

Adds the possibility for an adapter to receive references to other loaded adapters, so that it can potentially interact with those other adapters. It is designed so that this interaction is done via the GET and PUT methods as if done over HTTP, but the created ApiAdapterRequest object replaces the resource heavy HTTP request with a lightweight version that contains only what we need.

Closes #28

…ary of loaded adapters to be passed to each adapter by the API

…lace the full HTTP Requests used in Get and Put methods for internal Adapter Communication

…test the adapter communication

…d for the inter adapter communication

… the Put method of the target adapter via IAC

…e it's used in a couple places

… it does

ANeaves · 2019-03-11T13:46:12Z

Created this PR even though it can't be automatically merged. Not sure what the usual tactic here is so please let me know.

timcnicholls · 2019-03-11T13:50:17Z

You can rebase your branch off master and resolve the merge conflict, then force push the branch. Rebasing public branches is regarded as a bad thing, but it's OK on a feature branch that only you are working on.

You can also just merge from master, resolve the conflict and push again. I'm never convinced which is preferable.

coveralls · 2019-03-11T13:53:39Z

Coverage decreased (-0.09%) to 99.134% when pulling 150a904 on inter-adapter-communication into 1814ea7 on master.

ANeaves · 2019-03-11T13:56:35Z

The Merge Conflict was a part of the test_adapter.py file (since both me and @timcnicholls had modified the adapter class and thus the tests) but I've manually fixed that conflict and we are okay now. I was hesitant to rebase because I've broken things by doing that before and I'd rather not run the risk this time

timcnicholls · 2019-03-12T14:30:39Z

odin/adapters/adapter.py

+    def initialize(self, adapters):
+        """Initialize the ApiAdapter after it has been registered by the API Route.
+
+        This is an abstract implimentation of the initialize mechinism that allows


Typo on implementation

timcnicholls · 2019-03-12T14:33:58Z

odin/adapters/adapter.py

+class ApiAdapterRequest(object):
+    """API Adapter Request object.
+
+    Designed to emulate the HTTP Request Object used in the Get and Put requests


Suggest this should read

'Emulates the HTTPServerRequest' class passed as an argument to adapter HTTP verb methods (GET, PUT, ...)'

timcnicholls · 2019-03-12T14:40:49Z

odin/adapters/iac_dummy.py

@@ -0,0 +1,83 @@
+import logging


To avoid proliferation of test code source files in the main source tree I suggest we, as a minimum, merge iac_dummy.py and iac_dummy_target.py. They could also be merged into dummy.py.

timcnicholls · 2019-03-12T14:41:03Z

odin/adapters/iac_dummy_target.py

@@ -0,0 +1,60 @@
+import logging


See above comment

timcnicholls · 2019-03-12T14:42:02Z

odin/testing/adapters/test_iac_dummy.py

@@ -0,0 +1,49 @@
+from nose.tools import assert_equal


Ditto merging these sources into one file

timcnicholls · 2019-03-12T14:58:38Z

Thanks @ANeaves for implementing this. Looks good! Comments above are minor suggestions for improvement.

timcnicholls

As discussed, let's refactor location and use of IAC dummy adapters. We should also allow IAC on all 'core' adapters, i.e. system_info, system_status and proxy

…t structure

…e IAC Dummy Adapter is for

ANeaves · 2019-03-13T14:01:14Z

@timcnicholls I've merged the IAC dummy adapter into the original Dummy adapter. I also merged the tests, and removed the dummy target since it wasn't really any different from the dummy adapter that already existed.

timcnicholls

Looks good @ANeaves thanks for doing this. Have made a few very minor comments.

timcnicholls · 2019-03-14T09:32:06Z

odin/adapters/adapter.py

+    """API Adapter Request object.
+
+    Emulate the HTTPServerRequest class passed as an argument to adatper HTTP
+    verb methods(GET, PUT etc), for internal communication between adapters.


Missing space before (GET, ...

timcnicholls · 2019-03-14T09:32:34Z

odin/adapters/dummy.py

@@ -1,8 +1,12 @@
 """ Dummy adapter class for the ODIN server.


Should now be "classes"

timcnicholls · 2019-03-14T09:32:53Z

odin/adapters/dummy.py

@@ -1,8 +1,12 @@
 """ Dummy adapter class for the ODIN server.

-This class implements a dummy adapter for the ODIN server, demonstrating the
+The "DummyAdapter" class implements a dummy adapter for the ODIN server, demonstrating the


Can remove quotes from these docstrings

timcnicholls · 2019-03-14T09:33:26Z

odin/adapters/dummy.py

+        Call the get method of each other adapter that is loaded and return the responses
+        in a dictionary.
+        """
+        logging.debug("IAC DUMMY GET")


Shouty-shouty debug messages 😉

Debug's gotta get your attention somehow ;)

True, but that's through the sheer force of your coding brilliance, not gratuitous use of all-caps!

MattTaylorDLS · 2019-03-15T08:48:04Z

odin/adapters/adapter.py

+class ApiAdapterRequest(object):
+    """API Adapter Request object.
+
+    Emulate the HTTPServerRequest class passed as an argument to adatper HTTP


type: adatper

ulrikpedersen

Rubber stamp! (Apologies, I can't take too much of a close look)

I suggest at least @ajgdls should have a closer look as he is the one who works most with odin-control - and perhaps one of the other reviewers. We don't generally need the full gang for review...

GDYendell · 2019-03-15T09:27:09Z

odin/adapters/dummy.py

+        for key, value in self.adapters.items():
+            if value is self:
+                del self.adapters[key]
+                break


Suggested change

break

self.adapters = dict((k, v) for k, v in adapters.items() if v is not self)

Maybe?

ajgdls

Looks good, the changes weren't very intrusive to the existing infrastructure which is nice.

…pter initialize method to a tidier single line pythonic for loop

ANeaves added 17 commits February 27, 2019 13:17

Added initialize method to the base adapter class, to allow a diction…

bdeb95d

…ary of loaded adapters to be passed to each adapter by the API

Created the APIRequest Class in Adapter. This can then be used to rep…

28af8d6

…lace the full HTTP Requests used in Get and Put methods for internal Adapter Communication

Added set methids to apiAdapterRequest. Added some dummy adpaters to …

7f3faa6

…test the adapter communication

Added tests for changes to Adapter.py

30bf9a0

Added test to test_api for the initialize_adapters method

5ef8c1a

Created Util file. It contains the decode_request_body method require…

b030837

…d for the inter adapter communication

Modified the IAC Dummy adapters to use the util decode method and use…

522e53f

… the Put method of the target adapter via IAC

fixed test error

422d50c

Added param tree to IACdummy. Added remote_ip to AdapterRequest, sinc…

fc88b8b

…e it's used in a couple places

Added tests for util package. Renamed one util method to clarify what…

c42d9fd

… it does

Added tests for IAC Dummy Adapter

e3bea48

Added a test for the remote_ip added to the ApiAdapterRequest

ffb61b0

Fixed incompatability with Python 3

8e2dcab

Added tests for IAC Dummy Target. Removed redundant imports from test

6133268

Added and edited all docstring

2c3d692

Woops missed a method. More Docstring added

d5057e9

Consistency on strings. Only using double quotes now

b778950

ghost assigned ANeaves Mar 11, 2019

ghost added the in progress label Mar 11, 2019

ANeaves requested review from timcnicholls, ajgdls and ulrikpedersen March 11, 2019 13:45

Merge branch 'master' into inter-adapter-communication

aaed864

ANeaves requested review from GDYendell, MattTaylorDLS and skyfrench March 11, 2019 13:58

timcnicholls requested changes Mar 12, 2019

View reviewed changes

ANeaves added 2 commits March 13, 2019 13:25

Merged IAC test stuff in with the original dummy adapter file and tes…

a8a045a

…t structure

Modified the Docstring of the dummy adapter file, to describe what th…

ada1482

…e IAC Dummy Adapter is for

ANeaves requested a review from timcnicholls March 14, 2019 09:19

timcnicholls requested changes Mar 14, 2019

View reviewed changes

MattTaylorDLS reviewed Mar 15, 2019

View reviewed changes

ulrikpedersen approved these changes Mar 15, 2019

View reviewed changes

GDYendell approved these changes Mar 15, 2019

View reviewed changes

ajgdls approved these changes Mar 15, 2019

View reviewed changes

ANeaves added 2 commits March 15, 2019 13:06

Changed docstring typos following feedback. Modfied the IAC dummy ada…

255b219

…pter initialize method to a tidier single line pythonic for loop

All core adapters now accept the odin native request type by default

150a904

timcnicholls approved these changes Mar 18, 2019

View reviewed changes

ANeaves merged commit 01c51d7 into master Mar 18, 2019

ghost removed the in progress label Mar 18, 2019

ANeaves deleted the inter-adapter-communication branch March 18, 2019 08:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Inter adapter communication #36

Inter adapter communication #36

ANeaves commented Mar 11, 2019 •

edited

Loading

ANeaves commented Mar 11, 2019

timcnicholls commented Mar 11, 2019

coveralls commented Mar 11, 2019 •

edited

Loading

ANeaves commented Mar 11, 2019

timcnicholls Mar 12, 2019

timcnicholls Mar 12, 2019

timcnicholls Mar 12, 2019

timcnicholls Mar 12, 2019

timcnicholls Mar 12, 2019

timcnicholls commented Mar 12, 2019

timcnicholls left a comment

ANeaves commented Mar 13, 2019

timcnicholls left a comment

timcnicholls Mar 14, 2019

timcnicholls Mar 14, 2019

timcnicholls Mar 14, 2019

timcnicholls Mar 14, 2019

ANeaves Mar 14, 2019

timcnicholls Mar 14, 2019

ulrikpedersen Mar 15, 2019

MattTaylorDLS Mar 15, 2019

ulrikpedersen left a comment

GDYendell Mar 15, 2019 •

edited

Loading

ajgdls left a comment

		@@ -1,8 +1,12 @@
		""" Dummy adapter class for the ODIN server.

	break
	self.adapters = dict((k, v) for k, v in adapters.items() if v is not self)

Inter adapter communication #36

Inter adapter communication #36

Conversation

ANeaves commented Mar 11, 2019 • edited Loading

ANeaves commented Mar 11, 2019

timcnicholls commented Mar 11, 2019

coveralls commented Mar 11, 2019 • edited Loading

ANeaves commented Mar 11, 2019

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

timcnicholls commented Mar 12, 2019

timcnicholls left a comment

Choose a reason for hiding this comment

ANeaves commented Mar 13, 2019

timcnicholls 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

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

ulrikpedersen left a comment

Choose a reason for hiding this comment

GDYendell Mar 15, 2019 • edited Loading

Choose a reason for hiding this comment

ajgdls left a comment

Choose a reason for hiding this comment

ANeaves commented Mar 11, 2019 •

edited

Loading

coveralls commented Mar 11, 2019 •

edited

Loading

GDYendell Mar 15, 2019 •

edited

Loading