Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attempt at example for logging consistency for discussion #1221

Merged
merged 20 commits into from Nov 26, 2019
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
fe54efc
An quick run of connectors with a shot at rules around logging consis…
iguyking Oct 31, 2019
8ab220c
Merge branch 'master' into consistent_logging
iguyking Nov 3, 2019
46c618c
Shifting to period ending sentances
iguyking Nov 3, 2019
fa2e593
An quick run of connectors with a shot at rules around logging consis…
iguyking Oct 31, 2019
fce301f
Shifting to period ending sentances
iguyking Nov 3, 2019
4d3a16c
Merge branch 'consistent_logging' of github.com:iguyking/opsdroid int…
iguyking Nov 7, 2019
ecc0ec9
Another run for consistency
iguyking Nov 7, 2019
5b91c24
Merge remote-tracking branch 'upstream/master' into consistent_logging
iguyking Nov 18, 2019
315902c
Full run of logging messages
iguyking Nov 20, 2019
d56deea
Merge branch 'master' into consistent_logging
jacobtomlinson Nov 21, 2019
180aee4
Merge branch 'master' into consistent_logging
jacobtomlinson Nov 22, 2019
2bd4629
Black reformat
iguyking Nov 26, 2019
cb39d83
Merge branch 'master' into consistent_logging
iguyking Nov 26, 2019
ece095d
Update opsdroid/connector/github/__init__.py
iguyking Nov 26, 2019
eb5b122
Update opsdroid/connector/telegram/__init__.py
iguyking Nov 26, 2019
a2a8eda
more double quote cleanup
iguyking Nov 26, 2019
1ca11d3
Updated for black and test case fail
iguyking Nov 26, 2019
68a5be9
Merge branch 'master' into consistent_logging
FabioRosado Nov 26, 2019
a8c4b1a
Should be removed from previous PR
iguyking Nov 26, 2019
cd04520
Merge branch 'consistent_logging' of github.com:iguyking/opsdroid int…
iguyking Nov 26, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 12 additions & 0 deletions docs/contributing.md
Expand Up @@ -181,6 +181,18 @@ 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)
iguyking marked this conversation as resolved.
Show resolved Hide resolved
* All usage of the opsdroid capabilities should be Capitalized (Connector, Skill, Database and Parsers)
iguyking marked this conversation as resolved.
Show resolved Hide resolved
* All sentances should end with a period.
* When inserting specific text from an error message, end the sentance with a period.
* When inserting the entire error output as the last part of the log message, do not end it with a period.



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

Expand Down
8 changes: 4 additions & 4 deletions opsdroid/connector/facebook/__init__.py
Expand Up @@ -32,7 +32,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 @@ -100,7 +100,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 @@ -110,8 +110,8 @@ async def send_message(self, message):
async with aiohttp.ClientSession() 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."))
10 changes: 5 additions & 5 deletions opsdroid/connector/github/__init__.py
Expand Up @@ -18,12 +18,12 @@ 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")
_("Missing auth token!" "You must set 'token' in your config.")
iguyking marked this conversation as resolved.
Show resolved Hide resolved
)
self.name = self.config.get("name", "github")
self.opsdroid = opsdroid
Expand All @@ -35,7 +35,7 @@ async def connect(self):
async with aiohttp.ClientSession() 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 @@ -87,7 +87,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 @@ -97,7 +97,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
10 changes: 5 additions & 5 deletions opsdroid/connector/gitter/__init__.py
Expand Up @@ -19,7 +19,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 @@ -34,7 +34,7 @@ async def connect(self):
"""Create the connection."""

# Create connection object with chat library
_LOGGER.debug(_("Connecting with gitter stream"))
_LOGGER.debug(_("Connecting with Gitter stream."))
self.session = aiohttp.ClientSession()
gitter_url = self.build_url(
GITTER_STREAM_API,
Expand All @@ -56,7 +56,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 All @@ -82,7 +82,7 @@ async def parse_message(self, message):
message["text"], message["fromUser"]["username"], self.room_id, 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 @@ -98,7 +98,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 @@ -121,7 +121,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 @@ -130,14 +130,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 @@ -151,14 +151,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 @@ -46,8 +46,7 @@ def __init__(self, config, opsdroid=None):
except (KeyError, AttributeError):
_LOGGER.error(
_(
"Unable to login: Access token is missing. "
jacobtomlinson marked this conversation as resolved.
Show resolved Hide resolved
"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 @@ -82,10 +81,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 @@ -179,7 +178,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 @@ -191,9 +190,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 @@ -16,7 +16,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 @@ -102,14 +102,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 @@ -122,7 +121,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 @@ -21,7 +21,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 @@ -51,7 +51,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 @@ -70,14 +70,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."
iguyking marked this conversation as resolved.
Show resolved Hide resolved
"The Slack Connector will not be available."
),
error,
Expand Down Expand Up @@ -111,14 +111,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 @@ -135,7 +135,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 @@ -152,7 +152,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 @@ -169,7 +169,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 @@ -181,7 +181,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