Skip to content

Commit

Permalink
Merge pull request #51 from driplineorg/dl3/docs-coverage
Browse files Browse the repository at this point in the history
Dl3/docs coverage
  • Loading branch information
laroque authored Jan 14, 2020
2 parents 07b44bf + 9bbe599 commit 022b456
Show file tree
Hide file tree
Showing 14 changed files with 120 additions and 112 deletions.
13 changes: 7 additions & 6 deletions doc/contribute.rst
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,8 @@ Merging

Merging follows the instruction from the branching section above in most cases, but there are a few additional notes:

- If you'd like for someone to look over your changes before they are integrated, please push your branch and create a pull request
- If you are new to dripline-python, we ask that you use a PR for anything other than a very minor change when contributing to develop
- In the near term, we're planning to require PRs for all merges into the master branches when new releases are created
- With the default travis-ci config, if the name of your branch ends in ``\build``, then when you push it will trigger travis-ci tasks
- Anyone may offer contributions by forking and opening a PR, org members can push branches directly but are still expected to use PRs when contributing to the ``develop`` or ``master`` branches.

Coding Style
============
Expand All @@ -28,13 +27,15 @@ The internet is full of arguments over coding style which we're not really inter
- Regardless of what that style is, consistent styling makes code easier to read
- If we follow what is used in the python community then that is more intuitive to new people
- We're not robots and common sense can win out
- doc strings can be special depending on how they are expected to be rendered (for example in the API docs section of this page)
- doc strings can be special, depending on how they are expected to be rendered (for example in the API docs section of this page)

Based on that we prefer the following:

- Generally we try to follow `PEP-8 <https://www.python.org/dev/peps/pep-0008/>`_
- The 80 column limit makes things harder to read more of then easier (displays have had more than 80 columns and editors have been wrapping text for some time now). Use your judgement on when forced line-wrapping is needed
- The 80 column limit makes things harder to read more often than easier (displays have had more than 80 columns and editors have been wrapping text for some time now). Use your judgement on when forced line-wrapping is needed
- When creating new classes which inherit from local classes, decorate them with ``@fancy_doc`` so that the class's docstring gets updated

- The description of the class belongs in the class's doc string
- The doc string for the ``__init__`` method should just include description of the kwargs for that class (the decorator will add kwargs from inherited classes)
- The doc string for the ``__init__`` method should just include description of the kwargs for that class


10 changes: 8 additions & 2 deletions dripline/core/endpoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,15 @@

__all__.append('Endpoint')
class Endpoint(_Endpoint):
def __init__(self, name, calibration=None):
'''
Base class for all objects which can be sent dripline requests.
Every object described in a runtime configuration passed to `dl-serve` should derive from this class (either directly or indirectly).
'''
def __init__(self, name):
'''
name (str) : the name of the endpoint, specifies the binding key for request messages to which this object should reply
'''
_Endpoint.__init__(self, name)
self._calibration = calibration

def do_get_request(self, a_request_message):
print("in get_request")
Expand Down
7 changes: 6 additions & 1 deletion dripline/core/entity.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class Entity(Endpoint):
'''
#check_on_set -> allows for more complex logic to confirm successful value updates
# (for example, the success condition may be measuring another endpoint)
def __init__(self, get_on_set=False, log_routing_key_prefix='sensor_value', log_interval=0, log_on_set=False, **kwargs):
def __init__(self, get_on_set=False, log_routing_key_prefix='sensor_value', log_interval=0, log_on_set=False, calibration=None, **kwargs):
'''
get_on_set: if true, calls to on_set are immediately followed by an on_get, which is returned
log_routing_key_prefix: first term in routing key used in alert messages which log values
Expand All @@ -65,10 +65,15 @@ def __init__(self, get_on_set=False, log_routing_key_prefix='sensor_value', log_
to the datetime.time_delta initializer; if a datetime.timedelta taken as the new value
log_on_set: if true, always call log_a_value() immediately after on_set
**Note:** requires get_on_set be true, overrides must be equivalent
calibration (string || dict) : if string, updated with raw on_get() result via str.format() in
@calibrate decorator, used to populate raw and calibrated values
fields of a result payload. If a dictionary, the raw result is used
to index the dict with the calibrated value being the dict's value.
'''
Endpoint.__init__(self, **kwargs)

self.log_routing_key_prefix=log_routing_key_prefix
self._calibration = calibration

# keep a reference to the on_set (possibly decorated in a subclass), needed for changing *_on_set configurations
self.__initial_on_set = self.on_set
Expand Down
11 changes: 9 additions & 2 deletions dripline/core/throw_reply.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,17 @@

__all__.append('ThrowReply')
class ThrowReply(Exception):
'''
Exception class for use throughout the codebase for exceptions which do not fatally interrupt a running dripline service.
These exceptions are caught in the top of the message handling call stack and result in an reply message being sent indicating an error.
The preferred way to send a reply indicating an error (or warning) is to raise an instance of ThrowReply.
In cases where a dependency library throws a custom exception, or any piece of code throws a standard python exception, if the expected behavior is an error reply and continued service operation, that exception must be handled and ThrowReply raised instead.
'''
def __init__(self, return_code=DL_Success(), message=DL_Success().description, payload=scarab.Param()):
'''
return_code (_dripline.core.ReturnCode || string) : either a ReturnCode object, or the string name of a return code
message (string) : string to pass into the exception object, provides clarification of the particular exception
return_code (_dripline.core.ReturnCode || string) : instance of subclass of ReturnCode, or the string name of a return code;
it must be an instance which is already registered and is used to determine various header fields of the resulting dripline message.
message (string) : provides more specific information about the particular exception (beyond the fixed ReturnCode's description
payload (scarab.Param) : any data to include in the payload of the reply message (in a Warning, should match the normal success data)
'''
Exception.__init__(self, message)
Expand Down
1 change: 1 addition & 0 deletions module_bindings/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
set( PB_DRIPLINE_CORE_HEADERFILES
dripline_core/_endpoint.hh
dripline_core/_endpoint_trampoline.hh
dripline_core/binding_helpers.hh
dripline_core/core_pybind.hh
dripline_core/constants_pybind.hh
dripline_core/dripline_config_pybind.hh
Expand Down
5 changes: 5 additions & 0 deletions module_bindings/README.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
# General guides
- The python module is defined in a single source file, it runs exporter functions defined in header files which correspond one-to-one with source files in dripline-python; they have names ending in `_pybind.hh`.
- When needed, trampoline classes and extra functions in support of bindings should be defined in their own source and header files as needed.
- It is preferrable to try to achieve pythonic interfaces for bound classes; this means naming arguments and providing default values where possible and including doc strings.

# Items list convention
In python, if a module has an attribute named `__all__`, then that lists (as strings) the names of the objects to be imported when doing `from <module> import *`.
Our pattern is to use that style of import in `__init__.py` files to bring classes and other implementations into the namespace of a package, and so this is required for bound C++ as well.
Expand Down
47 changes: 16 additions & 31 deletions module_bindings/dripline_core/_endpoint_pybind.hh
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include "pybind11/pybind11.h"
#include "pybind11/iostream.h"

#include "binding_helpers.hh"
#include "endpoint.hh"

#include "_endpoint_trampoline.hh"
Expand All @@ -17,44 +18,28 @@ namespace dripline_pybind

all_items.push_back( "_Endpoint" );
pybind11::class_< dripline::endpoint, _endpoint_trampoline, std::shared_ptr< dripline::endpoint > >( mod, "_Endpoint", "Endpoint binding" )
.def( pybind11::init< const std::string& >(),
pybind11::call_guard< pybind11::scoped_ostream_redirect,
pybind11::scoped_estream_redirect >() )
.def( pybind11::init< const std::string& >(), DL_BIND_CALL_GUARD_STREAMS )

// mv_ properties
.def_property_readonly( "name", (std::string& (dripline::endpoint::*)()) &dripline::endpoint::name )
.def_property_readonly( "service", ( dripline::service_ptr_t& (dripline::endpoint::*)()) &dripline::endpoint::service )


// deal with messages
.def( "submit_request_message", &dripline::endpoint::submit_request_message,
pybind11::call_guard< pybind11::scoped_ostream_redirect,
pybind11::scoped_estream_redirect,
pybind11::gil_scoped_release >() )
.def( "on_request_message", &dripline::endpoint::on_request_message,
pybind11::call_guard< pybind11::scoped_ostream_redirect,
pybind11::scoped_estream_redirect,
pybind11::gil_scoped_release >() )
.def( "on_reply_message", &dripline::endpoint::on_reply_message,
pybind11::call_guard< pybind11::scoped_ostream_redirect,
pybind11::scoped_estream_redirect,
pybind11::gil_scoped_release >() )
.def( "on_alert_message", &dripline::endpoint::on_alert_message,
pybind11::call_guard< pybind11::scoped_ostream_redirect,
pybind11::scoped_estream_redirect,
pybind11::gil_scoped_release >() )
.def( "do_get_request", &dripline::endpoint::do_get_request,
pybind11::call_guard< pybind11::scoped_ostream_redirect,
pybind11::scoped_estream_redirect,
pybind11::gil_scoped_release >() )
.def( "do_set_request", &dripline::endpoint::do_set_request,
pybind11::call_guard< pybind11::scoped_ostream_redirect,
pybind11::scoped_estream_redirect,
pybind11::gil_scoped_release >() )
.def( "do_cmd_request", &dripline::endpoint::do_cmd_request,
pybind11::call_guard< pybind11::scoped_ostream_redirect,
pybind11::scoped_estream_redirect,
pybind11::gil_scoped_release >() )
.def( "submit_request_message", &dripline::endpoint::submit_request_message, DL_BIND_CALL_GUARD_STREAMS_AND_GIL,
"directly submits a request message to the endpoint for processing" )
.def( "on_request_message", &dripline::endpoint::on_request_message, DL_BIND_CALL_GUARD_STREAMS_AND_GIL,
"callback to execute when a new request message is recieved, has a stanard implementation but available for override")
.def( "on_reply_message", &dripline::endpoint::on_reply_message, DL_BIND_CALL_GUARD_STREAMS_AND_GIL,
"callback to execute when a new reply message is recieved, has a stanard implementation but available for override")
.def( "on_alert_message", &dripline::endpoint::on_alert_message, DL_BIND_CALL_GUARD_STREAMS_AND_GIL,
"callback to execute when a new alert message is recieved" )
.def( "do_get_request", &dripline::endpoint::do_get_request, DL_BIND_CALL_GUARD_STREAMS_AND_GIL,
"overridable method for implementing get handling behavior" )
.def( "do_set_request", &dripline::endpoint::do_set_request, DL_BIND_CALL_GUARD_STREAMS_AND_GIL,
"overridable method for implementing set handling behavior" )
.def( "do_cmd_request", &dripline::endpoint::do_cmd_request, DL_BIND_CALL_GUARD_STREAMS_AND_GIL,
"overridable method for implementing cmd handling behavior" )
;
return all_items;
}
Expand Down
10 changes: 10 additions & 0 deletions module_bindings/dripline_core/binding_helpers.hh
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#ifndef DRIPLINE_PYBIND_BINDING_HELPERS
#define DRIPLINE_PYBIND_BINDING_HELPERS

#define DL_BIND_CALL_GUARD_STREAMS \
pybind11::call_guard< pybind11::scoped_ostream_redirect, pybind11::scoped_estream_redirect >()

#define DL_BIND_CALL_GUARD_STREAMS_AND_GIL \
pybind11::call_guard< pybind11::scoped_ostream_redirect, pybind11::scoped_estream_redirect, pybind11::gil_scoped_release >()

#endif /* DRIPLINE_PYBIND_BINDING_HELPERS */
42 changes: 24 additions & 18 deletions module_bindings/dripline_core/message_pybind.hh
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#ifndef DRIPLINE_PYBIND_MESSAGE
#define DRIPLINE_PYBIND_MESSAGE

#include "binding_helpers.hh"

#include "message.hh"
#include "message_trampoline.hh"

Expand Down Expand Up @@ -29,10 +31,10 @@ namespace dripline_pybind
message
********/
all_items.push_back( "Message" );
pybind11::class_< dripline::message, message_trampoline, std::shared_ptr< dripline::message > > message( mod, "Message" );
pybind11::class_< dripline::message, message_trampoline, std::shared_ptr< dripline::message > > message( mod, "Message", "base class for all dripline messages" );

// internal types
pybind11::enum_< dripline::message::encoding >( message, "encoding" )
pybind11::enum_< dripline::message::encoding >( message, "encoding", "mime-type of message encoding" )
.value( "json", dripline::message::encoding::json )
;

Expand Down Expand Up @@ -78,26 +80,22 @@ namespace dripline_pybind
.def( "create_amqp_messages",
&dripline::message::create_amqp_messages,
"From message object to AMQP",
pybind11::call_guard< pybind11::scoped_ostream_redirect,
pybind11::scoped_estream_redirect >()
DL_BIND_CALL_GUARD_STREAMS
)
.def( "encode_message_body",
&dripline::message::encode_message_body,
"From message object to string",
pybind11::call_guard< pybind11::scoped_ostream_redirect,
pybind11::scoped_estream_redirect >()
DL_BIND_CALL_GUARD_STREAMS
)
.def( "derived_modify_amqp_message",
&_message::derived_modify_amqp_message,
"derived_modify_amqp_message function",
pybind11::call_guard< pybind11::scoped_ostream_redirect,
pybind11::scoped_estream_redirect >()
DL_BIND_CALL_GUARD_STREAMS
)
.def( "derived_modify_message_param",
&_message::derived_modify_message_param,
"derived_modify_amqp_message function",
pybind11::call_guard< pybind11::scoped_ostream_redirect,
pybind11::scoped_estream_redirect >()
DL_BIND_CALL_GUARD_STREAMS
)

.def( "encode_full_message", [](const dripline::message& a_message){ return a_message.encode_full_message(4000); } )
Expand All @@ -107,7 +105,8 @@ namespace dripline_pybind
msg_request
************/
all_items.push_back( "MsgRequest" );
pybind11::class_< dripline::msg_request, msg_request_trampoline, std::shared_ptr< dripline::msg_request > >( mod, "MsgRequest", message )
pybind11::class_< dripline::msg_request, msg_request_trampoline, std::shared_ptr< dripline::msg_request >
>( mod, "MsgRequest", message, "dripline messages containing a request to be sent to an endpoint" )
// constructor(s)
.def( pybind11::init< >() )
// properties
Expand Down Expand Up @@ -139,7 +138,8 @@ namespace dripline_pybind
pybind11::arg( "routing_key" ) = "",
pybind11::arg( "specifier" ) = "",
pybind11::arg( "reply_to" ) = "",
pybind11::arg( "encoding" ) = dripline::message::encoding::json
pybind11::arg( "encoding" ) = dripline::message::encoding::json,
"create and populate a new MsgRequest instance"
)
.def( "reply",
[](dripline::request_ptr_t a_req,
Expand All @@ -149,15 +149,17 @@ namespace dripline_pybind
{return a_req->reply( a_return_code, a_return_message, a_payload.clone() );},
pybind11::arg( "return_code" ) = 0,
pybind11::arg( "return_message" ) = "",
pybind11::arg_v( "payload", scarab::param(), "scarab::param()" )
pybind11::arg_v( "payload", scarab::param(), "scarab::param()" ),
"construct and send a reply message in response to this request"
)
;

/************
msg_reply
************/
all_items.push_back( "MsgReply" );
pybind11::class_< dripline::msg_reply, msg_reply_trampoline, std::shared_ptr< dripline::msg_reply > >( mod, "MsgReply", message )
pybind11::class_< dripline::msg_reply, msg_reply_trampoline, std::shared_ptr< dripline::msg_reply >
>( mod, "MsgReply", message, "dripline messages containing a reply to a previously received request" )
// constructor(s)
.def( pybind11::init< >() )

Expand All @@ -184,7 +186,8 @@ namespace dripline_pybind
pybind11::arg( "payload" ) = scarab::param(),
pybind11::arg( "routing_key" ) = "",
pybind11::arg( "specifier" ) = "",
pybind11::arg( "encoding" ) = dripline::message::encoding::json
pybind11::arg( "encoding" ) = dripline::message::encoding::json,
"create and populate a new MsgReply instance"
)
.def_static( "create",
[](unsigned a_retcode_value,
Expand All @@ -195,15 +198,17 @@ namespace dripline_pybind
pybind11::arg( "return_code" ) = 0,
pybind11::arg( "return_message" ) = "",
pybind11::arg( "payload" ) = scarab::param(),
pybind11::arg( "msg_request" ) = dripline::message::encoding::json
pybind11::arg( "msg_request" ) = dripline::message::encoding::json,
"create and populate a new MsgReply instance"
)
;

/***********
msg_alert
************/
all_items.push_back( "MsgAlert" );
pybind11::class_< dripline::msg_alert, msg_alert_trampoline, std::shared_ptr< dripline::msg_alert > >( mod, "MsgAlert", message )
pybind11::class_< dripline::msg_alert, msg_alert_trampoline, std::shared_ptr< dripline::msg_alert >
>( mod, "MsgAlert", message, "dripline message containing alert information" )
// constructor(s)
.def( pybind11::init< >() )

Expand All @@ -217,7 +222,8 @@ namespace dripline_pybind
pybind11::arg( "payload" ) = scarab::param(),
pybind11::arg( "routing_key" ) = "",
pybind11::arg( "specifier" ) = "",
pybind11::arg( "encoding" ) = dripline::message::encoding::json
pybind11::arg( "encoding" ) = dripline::message::encoding::json,
"create and populate a new MsgAlert instance"
)
;

Expand Down
Loading

0 comments on commit 022b456

Please sign in to comment.