[#643] AMQP Adapter documentation #728

Alfusainey · 2018-07-22T13:19:18Z

Documentation for the AMQP adapter (for the hono website)

@sophokles73: If i have #727 on master, i will rebase this PR on master

Signed-off-by: Alfusainey Jallow alf.jallow@gmail.com

sophokles73 · 2018-07-23T06:37:15Z

site/content/admin-guide/amqp-adapter-config.md

+| `HONO_AMQP_KEY_STORE_PATH`<br>`--hono.amqp.keyStorePath` | no | - | The absolute path to the Java key store containing the private key and certificate that the protocol adapter should use for authenticating to clients. Either this option or the `HONO_AMQP_KEY_PATH` and `HONO_AMQP_CERT_PATH` options need to be set in order to enable TLS secured connections with clients. The key store format can be either `JKS` or `PKCS12` indicated by a `.jks` or `.p12` file suffix respectively. |
+| `HONO_AMQP_MAX_PAYLOAD_SIZE`<br>`--hono.amqp.maxPayloadSize` | no | `2048` | The maximum allowed size of an incoming AMQP message's payload in bytes. When a client sends a message with a larger payload, the message is discarded and the connection to the client gets closed. |
+| `HONO_AMQP_NATIVE_TLS_REQUIRED`<br>`--hono.amqp.nativeTlsRequired` | no | `false` | The server will probe for OpenSLL on startup if a secure port is configured. By default, the server will fall back to the JVM's default SSL engine if not available. However, if set to `true`, the server will fail to start at all in this case. |
+| `HONO_AMQP_PORT`<br>`--hono.amqp.port` | no | `8883` | The secure port that the protocol adapter should listen on.<br>See [Port Configuration]({{< relref "#port-configuration" >}}) below for details. |


are you sure that the default is 8883?

sophokles73 · 2018-07-23T06:38:57Z

site/content/admin-guide/amqp-adapter-config.md

+
+The secure port will mostly be required for production scenarios. However, it might be desirable to expose a non-TLS secured port instead, e.g. for testing purposes. In any case, the non-secure port needs to be explicitly enabled either by
+
+- explicitly setting `HONO_AMQP_INSECURE_PORT` to a valid port number, or by


there doesn't seem to be such a property ...

when starting an insecure port, this property is used to determine which insecure port the server should bind to. The default is 4040 if this configurable property is not explicitly set.

This property is not contained in the table above ...

sophokles73 · 2018-07-23T06:43:42Z

site/content/user-guide/amqp-adapter.md

+weight = 215
+++
+
+The AMQP protocol adapter allows clients (devices or gateway components) supporting the AMQP 1.0 protocol to publish telemetry messages and events to Eclipse Hono&trade;'s Telemetry and Event endpoints. The AMQP adapter supports uploading telemetry messages and events using the `AT_LEAST_ONCE` delivery semantics. It also supports uploading telemetry messages using the `AT_MOST_ONCE` (fire and forget) delivery semantics. With `AT_MOST_ONCE` delivery, the client device receives the *ACCEPTED* disposition frame from the adapter (even if the message cannot be delivered to the downstream consumer peer). This is because with `AT_MOST_ONCE`, the client has indicated that it is not interested to know what happens to the message once it has being sent. The `AT_MOST_ONCE` delivery semantics is **not supported** for events because with events, clients MUST be notified regardless of weather the message was delivered to the downstream consumer or not (in the case of a failure).


I think you should try not to be too technical in this introductory paragraph. In particular, you might want to just explain that there are two ways to publish telemetry that differ in quality of service. Most readers will probably have a hard time following you through explanation of AMQP 1.0 protocol specifics at this point. So, can you try to be less technical while still explaining the different QoS?

yes, make sense 👍

sophokles73 · 2018-07-23T06:44:24Z

site/content/user-guide/amqp-adapter.md

+
+In the case of a server-side error, the adapter responds with a *RELEASED* disposition, which indicates to the client that the message was OK but it cannot be delivered due to a failure beyond the control of the client. In this case, the client should attempt to redeliver the message again.
+
+<!--more-->


this needs to go waaaaaayyy up. It is the separator between the first one or two sentences explaining what this page is about and the rest of the page ...

please move to line 7

sophokles73 · 2018-07-23T06:45:54Z

site/content/user-guide/amqp-adapter.md

+
+## Device Authentication
+
+By default, all Hono protocol adapters require clients (devices or gateway components) to authenticate during connection establishment. This is the preferred way for devices to publish data via protocol adapters. The AMQP adapter can be configured to *disallow* authentication for devices by setting the value of `HONO_AMQP_AUTHENTICATION_REQUIRED` to `false`. In this guide, we will give examples for publishing telemetry and events for *authenticated* and *unauthenticated* clients. We will see in the examples later that the main difference between publishing messages for authenticated and unauthenticated devices is in the format of the AMQP 1.0 **address**. For unauthenticated devices, the address should include the tenant and device identifier.


which AMQP 1.0 address are you talking about here`? The link's target address or a message's address?

yes, i was referring to the link address. i will update the doc to reflect this

sophokles73 · 2018-07-23T06:48:52Z

site/content/user-guide/amqp-adapter.md

+The AMQP adapter supports publishing of telemetry to Hono's Telemetry API. Telemetry messages are published using both `AT_LEAST_ONCE` and `AT_MOST_ONCE` delivery semantics.
+
+* Address: `telemetry`
+* Delivery: `AT_MOST_ONCE` or `AT_LEAST_ONCE`


These terms are not defined by AMQP 1.0. Instead, you need to define these delivery modes by means of presettled vs unsettled ...

sophokles73 · 2018-07-23T06:49:24Z

site/content/user-guide/amqp-adapter.md

+* Delivery: `AT_MOST_ONCE` or `AT_LEAST_ONCE`
+* Authentication: required
+* Message Header(s):
+  * (optional) Arbitrary headers (durable, ttl, priority, ...)


what about content-type? Is that optional?

yes it is optional

What happens to the message that is sent downstream when the device does not include one?

sophokles73 · 2018-07-23T06:51:04Z

site/content/user-guide/amqp-adapter.md

+
+## Publish an Event (authenticated Device)
+
+* Address: `event` or `e`


really? is e supported? is t also supported for telemetry?

yes, the telemetry/event case considers both.. see EndpointType#fromString

sophokles73 · 2018-07-23T06:52:06Z

site/content/user-guide/amqp-adapter.md

+
+Publish some JSON data for device `4711`:
+
+    $ qpid-send -b "localhost:4040" --connection-options "{username:sensor1@DEFAULT_TENANT, password:hono-secret, protocol:amqp1.0}" -a "'telemetry'" --content-string '{"temp": 5}'


I am quite sure that not many people will be familiar with qpid-send, you should at least include a link to the Apache Qpid project page to point them in the right direction

sophokles73 · 2018-07-23T06:52:30Z

site/content/user-guide/amqp-adapter.md

+
+| Name               | Location        | Type      | Description                                                     |
+| :----------------- | :-------------- | :-------- | :-------------------------------------------------------------- |
+| *orig_adapter*     | *application*   | *string*  | Contains the adapter's *type name* which can be used by downstream consumers to determine the protocol adapter that the message has been received over. The AMQP adapter's type name is `hono-amqp. |


do you actually include these?

yes, i think the generic AbstractProtocolAdapterBase#newMessage(...) implementation adds these for all protocol adapters (by default)

sophokles73 · 2018-07-27T14:58:39Z

@Alfusainey any news on this one?

Alfusainey · 2018-07-27T15:45:44Z

@sophokles73 : yes, sure! am currently working on the ITs, which is almost complete... i will update this branch tomorrow morning (once the ITs are out of the way).

Alfusainey · 2018-07-29T23:52:39Z

@sophokles73: PR updated! for purposes of demonstration, i created a simple command-line client for interacting with the amqp adapter. i couldn't find a suitable client to use for our purpose.

sophokles73 · 2018-07-30T06:24:21Z

site/content/user-guide/amqp-adapter.md

+
+In the case of a server-side error, the adapter responds with a *RELEASED* disposition, which indicates to the client that the message was OK but it cannot be delivered due to a failure beyond the control of the client. In this case, the client should attempt to redeliver the message again.
+
+<!--more-->


please move to line 7

sophokles73 · 2018-07-30T06:24:31Z

site/content/user-guide/amqp-adapter.md

@@ -3,28 +3,33 @@ title = "AMQP Adapter"
 weight = 215
 +++

-The AMQP protocol adapter allows clients (devices or gateway components) supporting the AMQP 1.0 protocol to publish telemetry messages and events to Eclipse Hono&trade;'s Telemetry and Event endpoints. The AMQP adapter supports uploading telemetry messages and events using the `AT_LEAST_ONCE` delivery semantics. It also supports uploading telemetry messages using the `AT_MOST_ONCE` (fire and forget) delivery semantics. With `AT_MOST_ONCE` delivery, the client device receives the *ACCEPTED* disposition frame from the adapter (even if the message cannot be delivered to the downstream consumer peer). This is because with `AT_MOST_ONCE`, the client has indicated that it is not interested to know what happens to the message once it has being sent. The `AT_MOST_ONCE` delivery semantics is **not supported** for events because with events, clients MUST be notified regardless of weather the message was delivered to the downstream consumer or not (in the case of a failure).
+The AMQP protocol adapter allow clients (devices or gateway components) supporting the AMQP 1.0 protocol to publish telemetry messages and events to Eclipse Hono&trade;'s Telemetry and Event endpoints. Telemetry messages are published using either `AT_MOST_ONCE` (presettled) or `AT_LEAST_ONCE` (unsettled) quality of service while Event messages are published using only `AT_LEAST_ONCE` quality of service. 


The second sentence should be rephrased to the client's view, e.g.

Clients can publish Telemetry messages using either ...

sophokles73 · 2018-07-30T06:27:33Z

site/content/user-guide/amqp-adapter.md


 <!--more-->

 ## Device Authentication

-By default, all Hono protocol adapters require clients (devices or gateway components) to authenticate during connection establishment. This is the preferred way for devices to publish data via protocol adapters. The AMQP adapter can be configured to *disallow* authentication for devices by setting the value of `HONO_AMQP_AUTHENTICATION_REQUIRED` to `false`. In this guide, we will give examples for publishing telemetry and events for *authenticated* and *unauthenticated* clients. We will see in the examples later that the main difference between publishing messages for authenticated and unauthenticated devices is in the format of the AMQP 1.0 **address**. For unauthenticated devices, the address should include the tenant and device identifier.
+By default, all Hono protocol adapters require clients (devices or gateway components) to authenticate during connection establishment. This is the preferred way for devices to publish data via protocol adapters. The AMQP adapter can be configured to *disallow* authentication for devices by setting the value of `HONO_AMQP_AUTHENTICATION_REQUIRED` to `false`. In this guide, we will give examples for publishing telemetry and events for *authenticated* and *unauthenticated* clients. We will see in the examples later that the main difference between publishing messages for authenticated and unauthenticated devices is in the format of the  ** message address**. For unauthenticated devices, the message address should include the tenant and device identifiers.


IMHO we can simply omit the two last sentences here and maybe add a note to the examples for unauthenticated devices ...

sophokles73 · 2018-07-30T06:28:51Z

site/content/user-guide/amqp-adapter.md

-By default, all Hono protocol adapters require clients (devices or gateway components) to authenticate during connection establishment. This is the preferred way for devices to publish data via protocol adapters. The AMQP adapter can be configured to *disallow* authentication for devices by setting the value of `HONO_AMQP_AUTHENTICATION_REQUIRED` to `false`. In this guide, we will give examples for publishing telemetry and events for *authenticated* and *unauthenticated* clients. We will see in the examples later that the main difference between publishing messages for authenticated and unauthenticated devices is in the format of the AMQP 1.0 **address**. For unauthenticated devices, the address should include the tenant and device identifier.
+By default, all Hono protocol adapters require clients (devices or gateway components) to authenticate during connection establishment. This is the preferred way for devices to publish data via protocol adapters. The AMQP adapter can be configured to *disallow* authentication for devices by setting the value of `HONO_AMQP_AUTHENTICATION_REQUIRED` to `false`. In this guide, we will give examples for publishing telemetry and events for *authenticated* and *unauthenticated* clients. We will see in the examples later that the main difference between publishing messages for authenticated and unauthenticated devices is in the format of the  ** message address**. For unauthenticated devices, the message address should include the tenant and device identifiers.
+
+For the purpose of demonstrating the usage of the AMQP adapter, the example deployment is shipped with a **minimal** command-line client to interact with the adapter. The client is package as a single executable JAR file and can be run directly on the command-line with *java -jar <jar-file>*. In the examples below, we will use this client to simulate a client device publishing messages to the adapter.


... client is packaged ...

where can the client be found/downloaded?

thanks.. I mentioned that it will be shipped with the example deployment. can we add the client to the example deployment? or should it be separate from Hono? the only new library added is the apache commons-cli package for defining and parsing command-line arguments

When we want to add new dependencies to third party libraries, we need to get approval from the Eclipse IP team first. Getting this for Apache Commons libs is usually no big deal. However, I haven't seen any of the code so far. Did you create a PR for the client yet?

IMHO it should go into the cli module where we already have put the example consumer as well.

No i did not create a PR yet. It is located on my github repo here.

IMHO it should go into the cli module where we already have put the example consumer as well.

Ok, i will put it there and create a PR. thanks!

sophokles73 · 2018-07-30T06:29:51Z

site/content/user-guide/amqp-adapter.md


 ### SASL PLAIN Authentication

-The AMQP adapter currently supports authenticating clients using a *username* and *password*. This means that clients needs to provide a *username* and a *password* when connecting to the AMQP adapter. The *username* must match the pattern [*auth-id@tenant*], e.g. `sensor1@DEFAULT_TENANT`. The adapter verifies the credentials provided by the client against the credentials that the [Credentials Service] ({{< relref "#credentials-service-connection-configuration" >}}) has on record for the device. If the credentials match, then authentication is successful and the client device can proceed to publish messages to Hono.
+The AMQP adapter currently supports authenticating clients using [SASL PLAIN](https://tools.ietf.org/html/rfc4616) (i.e a *username* and *password*). This means that clients needs to provide a *username* and a *password* when connecting to the AMQP adapter. If the adapter is configured for multi-tenancy (i.e `HONO_AMQP_SINGLE_TENANT` is set to `false`), then the *username* must match the pattern [*auth-id@tenant*], e.g. `sensor1@DEFAULT_TENANT`. Otherwise the `DEFAULT_TENANT` is assumed and the tenant-id can be omitted from the username.


... clients need to provide...

sophokles73 · 2018-07-30T06:31:08Z

site/content/user-guide/amqp-adapter.md

@@ -35,10 +40,11 @@ TODO

 ## Publish Telemetry Data (authenticated Device)

-The AMQP adapter supports publishing of telemetry to Hono's Telemetry API. Telemetry messages are published using both `AT_LEAST_ONCE` and `AT_MOST_ONCE` delivery semantics.
+The AMQP adapter supports publishing of telemetry data to Hono's Telemetry API. Telemetry messages are published using both `AT_LEAST_ONCE` and `AT_MOST_ONCE` delivery semantics.


No, Telemetry messages can be published using either ... or ....

sophokles73 · 2018-07-30T06:32:04Z

site/content/user-guide/amqp-adapter.md

-* Delivery: `AT_MOST_ONCE` or `AT_LEAST_ONCE`
+* Message Address: `telemetry` or `t`
+  * This refers to the `to` property of the message.
+* Settlement Mode: `presettled` or `unsettled`


which one belongs to which QoS?

sophokles73 · 2018-07-30T06:33:36Z

site/content/user-guide/amqp-adapter.md

+* Delivery: `AT_MOST_ONCE` or `AT_LEAST_ONCE`
+* Authentication: required
+* Message Header(s):
+  * (optional) Arbitrary headers (durable, ttl, priority, ...)


What happens to the message that is sent downstream when the device does not include one?

Alfusainey · 2018-07-30T12:50:39Z

What happens to the message that is sent downstream when the device does not include one?

should I add the default message headers and properties included to the message sent downstream (under the Downstream Meta Data heading)?

sophokles73 · 2018-07-30T14:40:22Z

should I add the default message headers and properties included to the message sent downstream (under the Downstream Meta Data heading)?

IMHO that would be very helpful (as it is with the other adapters) ...

Alfusainey · 2018-07-31T09:08:34Z

@sophokles73 patch updated with 5bc0fef. i will include the command-line client in the cli module and open a separate PR.

Alfusainey · 2018-08-02T12:26:38Z

@sophokles73: any feedback on this?

sophokles73 · 2018-08-02T12:46:31Z

@Alfusainey I am waiting for the PR for the command line client ...

Alfusainey · 2018-08-02T13:05:18Z

humm... looks like we were waiting in cycles :). i will create the PR for the command-line client next

sophokles73 · 2018-08-04T07:46:50Z

@Alfusainey with #755 merged, you should finish this one next ...

Alfusainey · 2018-08-04T14:46:43Z

@sophokles73 PR updated with 2c9263a

sophokles73 · 2018-08-04T15:39:45Z

site/content/user-guide/amqp-adapter.md

@@ -38,6 +35,34 @@ There is a subtle difference between the *device identifier* (*device-id*) and t
 ### SASL EXTERNAL Authentication
 TODO

+## AMQP Command-line Client
+For the purposes of demonstrating the usage of the AMQP adapter, the **Hono CLI Module** contains an AMQP command-line client for interacting with the AMQP adapter. This module does not activate the AMQP client by default and should be activated using the `--spring.profile.active` argument.


simply remove the last sentence, it will just confuse readers.

sophokles73 · 2018-08-04T15:40:40Z

site/content/user-guide/amqp-adapter.md

+For the purposes of demonstrating the usage of the AMQP adapter, the **Hono CLI Module** contains an AMQP command-line client for interacting with the AMQP adapter. This module does not activate the AMQP client by default and should be activated using the `--spring.profile.active` argument.
+
+The command-line client contains the following arguments (with default values):
+* `message.address`: The AMQP 1.0 message address (default: `telemetry/DEFAULT_TENANT/4711`)


why don't all params include the double dashes, i.e. --message.address? The --spring.profiles.active does ...

sophokles73 · 2018-08-04T15:41:17Z

site/content/user-guide/amqp-adapter.md

+* `username`: The username to authenticate to the adapter (default: `sensor1@DEFAULT_TENANT`)
+* `password`: The password to authenticate to the adapter (default: `hono-secret`)
+* `payload`: The message payload body (default: `'{"temp": 5}'`)
+* `--spring.profiles.active=amqp-adapter-cli`: Tells Hono CLI to activate the AMQP command-line client. This argument does not contain a default value and must be provided when running the client on the command-line.


it does contain a default, but the default is to run the consumer ...

yes, sure.. i think it will be better to remove the last sentence

sophokles73 · 2018-08-04T15:44:18Z

site/content/user-guide/amqp-adapter.md

@@ -95,9 +120,21 @@ Note how verbose the address is for unauthenticated devices. This address can be

 Publish some JSON data for device `4711`:

-    $ java -jar AmqpSend.jar -b "localhost:4040" -to "telemetry/DEFAULT_TENANT/4711" -m '{"temp": 5}'
+    $ /hono/cli/target$ java -jar hono-cli-0.7-SNAPSHOT-exec.jar --spring.profiles.active=amqp-adapter-cli --username="" --password=""


are you sure this works? IIRC the command line client only checks the username and password fields to not be null ...

yes, thanks for spotting this.. am reading your comments just now (after already having created a PR to fix this for the client). i made the changes locally but included it in a different patch

Alfusainey · 2018-08-04T20:06:32Z

thanks @sophokles73. patch updated with 23a1bcb

sophokles73 · 2018-08-05T06:00:26Z

site/content/admin-guide/amqp-adapter-config.md

+
+The secure port will mostly be required for production scenarios. However, it might be desirable to expose a non-TLS secured port instead, e.g. for testing purposes. In any case, the non-secure port needs to be explicitly enabled either by
+
+- explicitly setting `HONO_AMQP_INSECURE_PORT` to a valid port number, or by


This property is not contained in the table above ...

Alfusainey · 2018-08-05T09:25:38Z

@sophokles73 PR updated with 518d8bb

Signed-off-by: Alfusainey Jallow <alf.jallow@gmail.com>

sophokles73 · 2018-08-05T09:50:21Z

@Alfusainey can you rebase this PR so that we can see if the Travis build passes?

Alfusainey · 2018-08-05T09:51:31Z

@sophokles73: i just rebased and squashed

sophokles73 requested changes Jul 23, 2018

View reviewed changes

Alfusainey force-pushed the amqp-documentation branch from 0d4d191 to 60531e8 Compare July 29, 2018 22:52

sophokles73 requested changes Jul 30, 2018

View reviewed changes

sophokles73 added AMQP Adapter documentation labels Aug 4, 2018

sophokles73 added this to To do in 0.7 via automation Aug 4, 2018

sophokles73 added this to the 0.7 milestone Aug 4, 2018

sophokles73 moved this from To do to In progress in 0.7 Aug 4, 2018

Alfusainey force-pushed the amqp-documentation branch from 5bc0fef to 2c9263a Compare August 4, 2018 14:41

sophokles73 reviewed Aug 4, 2018

View reviewed changes

sophokles73 requested changes Aug 5, 2018

View reviewed changes

sophokles73 approved these changes Aug 5, 2018

View reviewed changes

[eclipse-hono#643] AMQP Adapter documentation for the Hono Website

969c19b

Signed-off-by: Alfusainey Jallow <alf.jallow@gmail.com>

Alfusainey force-pushed the amqp-documentation branch from 518d8bb to 969c19b Compare August 5, 2018 09:50

sophokles73 merged commit ba7f62b into eclipse-hono:master Aug 5, 2018

0.7 automation moved this from In progress to Done Aug 5, 2018


		The secure port will mostly be required for production scenarios. However, it might be desirable to expose a non-TLS secured port instead, e.g. for testing purposes. In any case, the non-secure port needs to be explicitly enabled either by

		- explicitly setting `HONO_AMQP_INSECURE_PORT` to a valid port number, or by


		In the case of a server-side error, the adapter responds with a RELEASED disposition, which indicates to the client that the message was OK but it cannot be delivered due to a failure beyond the control of the client. In this case, the client should attempt to redeliver the message again.

		<!--more-->


		## Device Authentication

		By default, all Hono protocol adapters require clients (devices or gateway components) to authenticate during connection establishment. This is the preferred way for devices to publish data via protocol adapters. The AMQP adapter can be configured to disallow authentication for devices by setting the value of `HONO_AMQP_AUTHENTICATION_REQUIRED` to `false`. In this guide, we will give examples for publishing telemetry and events for authenticated and unauthenticated clients. We will see in the examples later that the main difference between publishing messages for authenticated and unauthenticated devices is in the format of the AMQP 1.0 address. For unauthenticated devices, the address should include the tenant and device identifier.


		## Publish an Event (authenticated Device)

		* Address: `event` or `e`


		Publish some JSON data for device `4711`:

		$ qpid-send -b "localhost:4040" --connection-options "{username:sensor1@DEFAULT_TENANT, password:hono-secret, protocol:amqp1.0}" -a "'telemetry'" --content-string '{"temp": 5}'

[#643] AMQP Adapter documentation #728

[#643] AMQP Adapter documentation #728

Conversation

Alfusainey commented Jul 22, 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

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

sophokles73 commented Jul 27, 2018

Alfusainey commented Jul 27, 2018

Alfusainey commented Jul 29, 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

Choose a reason for hiding this comment

Alfusainey commented Jul 30, 2018

sophokles73 commented Jul 30, 2018

Alfusainey commented Jul 31, 2018

Alfusainey commented Aug 2, 2018

sophokles73 commented Aug 2, 2018

Alfusainey commented Aug 2, 2018

sophokles73 commented Aug 4, 2018

Alfusainey commented Aug 4, 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

Alfusainey commented Aug 4, 2018

Choose a reason for hiding this comment

Alfusainey commented Aug 5, 2018

sophokles73 commented Aug 5, 2018

Alfusainey commented Aug 5, 2018