Skip to content

Commit

Permalink
Make log messages more consistent and add guidelines (#1221)
Browse files Browse the repository at this point in the history
* An quick run of connectors with a shot at rules around logging consistency for #1137

* Shifting to period ending sentances

* An quick run of connectors with a shot at rules around logging consistency for #1137

* Shifting to period ending sentances

* Another run for consistency

* Full run of logging messages

* Black reformat

* Update opsdroid/connector/github/__init__.py

Fix

Co-Authored-By: Fábio Rosado <fabioglrosado@gmail.com>

* Update opsdroid/connector/telegram/__init__.py

Double Quotes Cleanup

Co-Authored-By: Fábio Rosado <fabioglrosado@gmail.com>

* more double quote cleanup

* Updated for black and test case fail

* Should be removed from previous PR
  • Loading branch information
iguyking authored and jacobtomlinson committed Nov 26, 2019
1 parent 473b540 commit 8d36a33
Show file tree
Hide file tree
Showing 28 changed files with 163 additions and 161 deletions.
11 changes: 11 additions & 0 deletions docs/contributing.md
Expand Up @@ -181,6 +181,17 @@ Opsdroid is an open source chatbot framework. It is designed to be extendable, s

If you don't know where to start, make sure to check the [tutorials](tutorials/introduction.md) and read the [documentation](http://opsdroid.readthedocs.io/en/latest/?badge=latest). Remember that you can also ask for help in our [matrix channel](https://riot.im/app/#/room/#opsdroid-general:matrix.org)

## Creating Consistent Logging Messages

When creating log messages, we would like to see a consistency around how to structure and utilize them.

* In general any copyright/legal name should be Capitalized (i.e. Slack, Telegram)
* All usage of the opsdroid capabilities should be Capitalized (Connector, Skill, Database and Parsers)
* All sentances should end with a period.
* When inserting specific text from an error message, end the sentance with a period.



## Issues
You can help us by reporting new issues or by fixing existing issues.

Expand Down
10 changes: 4 additions & 6 deletions opsdroid/cli/utils.py
Expand Up @@ -142,19 +142,17 @@ def welcome_message(config):
_LOGGER.info("=" * 40)
_LOGGER.info(
_(
"You can customise your opsdroid by modifying "
"your configuration.yaml"
"You can customise your opsdroid by modifying your configuration.yaml."
)
)
_LOGGER.info(
_("Read more at: " "http://opsdroid.readthedocs.io/#configuration")
_("Read more at: http://opsdroid.readthedocs.io/#configuration")
)
_LOGGER.info(_("Watch the Get Started Videos at: " "http://bit.ly/2fnC0Fh"))
_LOGGER.info(_("Watch the Get Started Videos at: http://bit.ly/2fnC0Fh"))
_LOGGER.info(
_(
"Install Opsdroid Desktop at: \n"
"https://github.com/opsdroid/opsdroid-desktop/"
"releases"
"https://github.com/opsdroid/opsdroid-desktop/releases"
)
)
_LOGGER.info("=" * 40)
Expand Down
8 changes: 4 additions & 4 deletions opsdroid/connector/facebook/__init__.py
Expand Up @@ -39,7 +39,7 @@ class ConnectorFacebook(Connector):
def __init__(self, config, opsdroid=None):
"""Connector Setup."""
super().__init__(config, opsdroid=opsdroid)
_LOGGER.debug(_("Starting facebook connector"))
_LOGGER.debug(_("Starting Facebook Connector."))
self.name = self.config.get("name", "facebook")
self.bot_name = config.get("bot-name", "opsdroid")

Expand Down Expand Up @@ -107,7 +107,7 @@ async def listen(self):
@register_event(Message)
async def send_message(self, message):
"""Respond with a message."""
_LOGGER.debug(_("Responding to facebook"))
_LOGGER.debug(_("Responding to Facebook."))
url = _FACEBOOK_SEND_URL.format(self.config.get("page-access-token"))
headers = {"content-type": "application/json"}
payload = {
Expand All @@ -117,8 +117,8 @@ async def send_message(self, message):
async with aiohttp.ClientSession(trust_env=True) as session:
resp = await session.post(url, data=json.dumps(payload), headers=headers)
if resp.status < 300:
_LOGGER.info(_("Responded with: %s"), message.text)
_LOGGER.info(_("Responded with: %s."), message.text)
else:
_LOGGER.debug(resp.status)
_LOGGER.debug(await resp.text())
_LOGGER.error(_("Unable to respond to facebook"))
_LOGGER.error(_("Unable to respond to Facebook."))
12 changes: 5 additions & 7 deletions opsdroid/connector/github/__init__.py
Expand Up @@ -21,13 +21,11 @@ class ConnectorGitHub(Connector):
def __init__(self, config, opsdroid=None):
"""Create the connector."""
super().__init__(config, opsdroid=opsdroid)
logging.debug("Loaded GitHub connector")
logging.debug("Loaded GitHub connector.")
try:
self.github_token = config["token"]
except KeyError:
_LOGGER.error(
_("Missing auth token!" "You must set 'token' in your config")
)
_LOGGER.error(_("Missing auth token! You must set 'token' in your config."))
self.name = self.config.get("name", "github")
self.opsdroid = opsdroid
self.github_username = None
Expand All @@ -38,7 +36,7 @@ async def connect(self):
async with aiohttp.ClientSession(trust_env=True) as session:
response = await session.get(url)
if response.status >= 300:
_LOGGER.error(_("Error connecting to github: %s"), response.text())
_LOGGER.error(_("Error connecting to GitHub: %s."), response.text())
return False
_LOGGER.debug(_("Reading bot information..."))
bot_data = await response.json()
Expand Down Expand Up @@ -94,7 +92,7 @@ async def github_message_handler(self, request):
)
await self.opsdroid.parse(message)
except KeyError as error:
_LOGGER.error(_("Key %s not found in payload"), error)
_LOGGER.error(_("Key %s not found in payload."), error)
_LOGGER.debug(payload)
return aiohttp.web.Response(text=json.dumps("Received"), status=201)

Expand All @@ -104,7 +102,7 @@ async def send_message(self, message):
# stop immediately if the message is from the bot itself.
if message.user == self.github_username:
return True
_LOGGER.debug(_("Responding via GitHub"))
_LOGGER.debug(_("Responding via GitHub."))
repo, issue = message.target.split("#")
url = "{}/repos/{}/issues/{}/comments".format(GITHUB_API_URL, repo, issue)
headers = {"Authorization": " token {}".format(self.github_token)}
Expand Down
14 changes: 8 additions & 6 deletions opsdroid/connector/gitter/__init__.py
Expand Up @@ -21,7 +21,7 @@ class ConnectorGitter(Connector):
def __init__(self, config, opsdroid=None):
"""Create the connector."""
super().__init__(config, opsdroid=opsdroid)
_LOGGER.debug(_("Starting Gitter connector"))
_LOGGER.debug(_("Starting Gitter Connector."))
self.name = "gitter"
self.session = None
self.response = None
Expand All @@ -36,8 +36,10 @@ async def connect(self):
"""Create the connection."""

# Create connection object with chat library
_LOGGER.debug(_("Connecting with gitter stream"))
self.session = aiohttp.ClientSession(trust_env=True)

_LOGGER.debug(_("Connecting with Gitter stream."))
self.session = aiohttp.ClientSession()

gitter_url = self.build_url(
GITTER_STREAM_API,
self.room_id,
Expand All @@ -58,7 +60,7 @@ def build_url(self, base_url, *res, **params):

async def listen(self):
"""Keep listing to the gitter channel."""
_LOGGER.debug(_("Listening with gitter stream"))
_LOGGER.debug(_("Listening with Gitter stream."))
while self.listening:
try:
await self._get_messages()
Expand Down Expand Up @@ -87,7 +89,7 @@ async def parse_message(self, message):
connector=self,
)
except KeyError as err:
_LOGGER.error(_("Unable to parse message %s"), err)
_LOGGER.error(_("Unable to parse message %s."), err)
_LOGGER.error(err)

@register_event(Message)
Expand All @@ -103,7 +105,7 @@ async def send_message(self, message):
payload = {"text": message.text}
resp = await self.session.post(url, json=payload, headers=headers)
if resp.status == 200:
_LOGGER.info(_("Successfully responded"))
_LOGGER.info(_("Successfully responded."))
else:
_LOGGER.error(_("Unable to respond."))

Expand Down
12 changes: 6 additions & 6 deletions opsdroid/connector/matrix/connector.py
Expand Up @@ -130,7 +130,7 @@ async def listen(self): # pragma: no cover
timeout_ms=int(60 * 1e3), # 1m in ms
filter=self.filter_id,
)
_LOGGER.debug(_("matrix sync request returned"))
_LOGGER.debug(_("Matrix sync request returned."))
message = await self._parse_sync_response(response)
if message:
await self.opsdroid.parse(message)
Expand All @@ -139,14 +139,14 @@ async def listen(self): # pragma: no cover
# We can safely ignore timeout errors. The non-standard error
# codes are returned by Cloudflare.
if mre.code in [504, 522, 524]:
_LOGGER.info(_("Matrix Sync Timeout (code: %d)"), mre.code)
_LOGGER.info(_("Matrix sync timeout (code: %d)."), mre.code)
continue

_LOGGER.exception(_("Matrix Sync Error"))
_LOGGER.exception(_("Matrix sync error."))
except CancelledError:
raise
except Exception: # pylint: disable=W0703
_LOGGER.exception(_("Matrix Sync Error"))
_LOGGER.exception(_("Matrix sync error."))

async def get_nick(self, roomid, mxid):
"""
Expand All @@ -160,14 +160,14 @@ async def get_nick(self, roomid, mxid):
return await self.connection.get_room_displayname(roomid, mxid)
except Exception: # pylint: disable=W0703
# Fallback to the non-room specific one
logging.exception("Failed to lookup room specific nick for %s", mxid)
logging.exception("Failed to lookup room specific nick for %s.", mxid)

try:
return await self.connection.get_display_name(mxid)
except MatrixRequestError as mre:
# Log the error if it's not the 404 from the user not having a nick
if mre.code != 404:
logging.exception("Failed to lookup nick for %s", mxid)
logging.exception("Failed to lookup nick for %s.", mxid)
return mxid

@staticmethod
Expand Down
13 changes: 6 additions & 7 deletions opsdroid/connector/rocketchat/__init__.py
Expand Up @@ -57,8 +57,7 @@ def __init__(self, config, opsdroid=None):
except (KeyError, AttributeError):
_LOGGER.error(
_(
"Unable to login: Access token is missing. "
"Rocket.Chat connector will not be available."
"Unable to login: Access token is missing. Rocket.Chat connector will not be available."
)
)

Expand Down Expand Up @@ -93,10 +92,10 @@ async def connect(self):
resp = await self.session.get(self.build_url("me"), headers=self.headers)
if resp.status != 200:
_LOGGER.error(_("Unable to connect."))
_LOGGER.error(_("Rocket.Chat error %s, %s"), resp.status, resp.text)
_LOGGER.error(_("Rocket.Chat error %s, %s."), resp.status, resp.text)
else:
json = await resp.json()
_LOGGER.debug(_("Connected to Rocket.Chat as %s"), json["username"])
_LOGGER.debug(_("Connected to Rocket.Chat as %s."), json["username"])

async def _parse_message(self, response):
"""Parse the message received.
Expand Down Expand Up @@ -191,7 +190,7 @@ async def send_message(self, message):
message (object): An instance of Message
"""
_LOGGER.debug(_("Responding with: %s"), message.text)
_LOGGER.debug(_("Responding with: %s."), message.text)

data = {}
data["channel"] = message.target
Expand All @@ -203,9 +202,9 @@ async def send_message(self, message):
)

if resp.status == 200:
_LOGGER.debug(_("Successfully responded"))
_LOGGER.debug(_("Successfully responded."))
else:
_LOGGER.debug(_("Error - %s: Unable to respond"), resp.status)
_LOGGER.debug(_("Error - %s: Unable to respond."), resp.status)

async def disconnect(self):
"""Disconnect from Rocket.Chat.
Expand Down
9 changes: 4 additions & 5 deletions opsdroid/connector/shell/__init__.py
Expand Up @@ -17,7 +17,7 @@ class ConnectorShell(Connector):

def __init__(self, config, opsdroid=None):
"""Create the connector."""
_LOGGER.debug(_("Loaded shell connector"))
_LOGGER.debug(_("Loaded shell Connector."))
super().__init__(config, opsdroid=opsdroid)
self.name = "shell"
self.config = config
Expand Down Expand Up @@ -103,14 +103,13 @@ async def connect(self):
"""
if platform.system() == "Windows":
_LOGGER.warning(
"The shell connector does not work on windows."
" Please install the Opsdroid Desktop App."
"The shell connector does not work on windows. Please install the Opsdroid Desktop App."
)
pass

async def listen(self):
"""Listen for and parse new user input."""
_LOGGER.debug(_("Connecting to shell"))
_LOGGER.debug(_("Connecting to shell."))
message_processor = self.loop.create_task(self._parse_message())
await self._closing.wait()
message_processor.cancel()
Expand All @@ -123,7 +122,7 @@ async def respond(self, message):
message (object): An instance of Message
"""
_LOGGER.debug(_("Responding with: %s"), message.text)
_LOGGER.debug(_("Responding with: %s."), message.text)
self.clear_prompt()
print(message.text)
self.draw_prompt()
Expand Down
26 changes: 13 additions & 13 deletions opsdroid/connector/slack/__init__.py
Expand Up @@ -31,7 +31,7 @@ class ConnectorSlack(Connector):
def __init__(self, config, opsdroid=None):
"""Create the connector."""
super().__init__(config, opsdroid=opsdroid)
_LOGGER.debug(_("Starting Slack connector"))
_LOGGER.debug(_("Starting Slack connector."))
self.name = "slack"
self.default_target = config.get("default-room", "#general")
self.icon_emoji = config.get("icon-emoji", ":robot_face:")
Expand Down Expand Up @@ -67,7 +67,7 @@ def __init__(self, config, opsdroid=None):

async def connect(self):
"""Connect to the chat service."""
_LOGGER.info(_("Connecting to Slack"))
_LOGGER.info(_("Connecting to Slack."))

try:
# The slack library recommends you call `self.slack_rtm.start()`` here but it
Expand All @@ -86,14 +86,14 @@ async def connect(self):
).data
self.bot_id = self.user_info["user"]["profile"]["bot_id"]

_LOGGER.debug(_("Connected as %s"), self.bot_name)
_LOGGER.debug(_("Using icon %s"), self.icon_emoji)
_LOGGER.debug(_("Default room is %s"), self.default_target)
_LOGGER.info(_("Connected successfully"))
_LOGGER.debug(_("Connected as %s."), self.bot_name)
_LOGGER.debug(_("Using icon %s."), self.icon_emoji)
_LOGGER.debug(_("Default room is %s."), self.default_target)
_LOGGER.info(_("Connected successfully."))
except slack.errors.SlackApiError as error:
_LOGGER.error(
_(
"Unable to connect to Slack due to %s - "
"Unable to connect to Slack due to %s."
"The Slack Connector will not be available."
),
error,
Expand Down Expand Up @@ -127,14 +127,14 @@ async def process_message(self, **payload):
return

# Lookup username
_LOGGER.debug(_("Looking up sender username"))
_LOGGER.debug(_("Looking up sender username."))
try:
user_info = await self.lookup_username(message["user"])
except ValueError:
return

# Replace usernames in the message
_LOGGER.debug(_("Replacing userids in message with usernames"))
_LOGGER.debug(_("Replacing userids in message with usernames."))
message["text"] = await self.replace_usernames(message["text"])

await self.opsdroid.parse(
Expand All @@ -151,7 +151,7 @@ async def process_message(self, **payload):
async def send_message(self, message):
"""Respond with a message."""
_LOGGER.debug(
_("Responding with: '%s' in room %s"), message.text, message.target
_("Responding with: '%s' in room %s."), message.text, message.target
)
await self.slack.api_call(
"chat.postMessage",
Expand All @@ -168,7 +168,7 @@ async def send_message(self, message):
async def send_blocks(self, blocks):
"""Respond with structured blocks."""
_LOGGER.debug(
_("Responding with interactive blocks in room %s"), blocks.target
_("Responding with interactive blocks in room %s."), blocks.target
)
await self.slack.api_call(
"chat.postMessage",
Expand All @@ -185,7 +185,7 @@ async def send_blocks(self, blocks):
async def send_reaction(self, reaction):
"""React to a message."""
emoji = demojize(reaction.emoji).replace(":", "")
_LOGGER.debug(_("Reacting with: %s"), emoji)
_LOGGER.debug(_("Reacting with: %s."), emoji)
try:
await self.slack.api_call(
"reactions.add",
Expand All @@ -197,7 +197,7 @@ async def send_reaction(self, reaction):
)
except slack.errors.SlackApiError as error:
if "invalid_name" in str(error):
_LOGGER.warning(_("Slack does not support the emoji %s"), emoji)
_LOGGER.warning(_("Slack does not support the emoji %s."), emoji)
else:
raise

Expand Down

0 comments on commit 8d36a33

Please sign in to comment.