Skip to content
This repository has been archived by the owner on Dec 16, 2019. It is now read-only.

Make everything validate flake8-docstrings #357

Merged
merged 3 commits into from
Sep 20, 2018
Merged

Conversation

blankdots
Copy link

A continuation of this PR #350

It might look like a lot has changed, however this PR add missing documentation to function, classes and makes sure every one of them adheres to the PEP 257 docstring conventions.

Ignored rules (a bit biased on them, so we can discuss if we should ignore more or less): E226,D203,D212,D213,D404,D100,D104

Documentation of the functions is minimal and any changes or suggestion to the descriptions are welcomed.

@blankdots blankdots self-assigned this Sep 19, 2018
@blankdots blankdots added this to the Sprint 36 milestone Sep 19, 2018
Copy link
Member

@viklund viklund left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found a few missplellings.

lega/utils/db.py Outdated
def mark_completed(file_id):
"""Mark file in completed."""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be "Mark file completed" (possibly "as completed").

def __str__(self): # Informal description
"""Return readale informal description."""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

readable with a b. Applies to all of the __str__ methods in this file.

class JSONFormatter(Formatter):
"""Json Logs formattingself.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

formattingself?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auto complete back stabbed me me on this.


def __init__(self, s3, bucket, path, mode='rb', blocksize=1 << 22): # 1<<22 = 4194304 = 4MB
"""Initialise class."""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm more a fan of american spelling (with a z), but whatever.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok.

lega/ingest.py Outdated
@db.catch_error
@db.crypt4gh_to_user_errors
def work(fs, channel, data):
'''Reads a message, splits the header and sends the remainder to the backend store.'''

"""Read a message, splits the header and sends the remainder to the backend store."""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new text is inconsistent.

@routes.get('/active/{key_type}')
async def retrieve_active_key(request):
"""Retrive the active key from the cache and serve it via HTTPS."""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: Retrieve

@@ -179,15 +189,15 @@ def _unlock_key(name, active=None, path=None, expire=None, passphrase=None, **kw

@routes.get('/admin/ttl')
async def check_ttl(request):
"""Evict from the cache if TTL expired
and return the keys that survived""" # ehh...why? /Fred
"""Evict from the cache if TTL expired and return the keys that survived.""" # ehh...why? /Fred
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stefan, can you address the comment at the end?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That endpoint is something we thought of when designing the keyserver and we kept it there.
the use case was to see which keys are active.

It is not subject of this PR, but most likely it will be removed as it is deprecated.

lega/utils/db.py Outdated
######################################
# DB connection #
######################################
def fetch_args(d):
"""Initializing a connection to db."""
"""Initialize a connection to db."""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it doesn't

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the log below started with Initializing a connection to... and got me confused.

@@ -20,33 +18,39 @@ class FileStorage():
"""Vault storage on disk and related I/O."""

def __init__(self):
"""Initialize POSIX storage at a path."""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initialize backend storage to a POSIX file system's path

@blankdots blankdots merged commit 92c4dd0 into dev Sep 20, 2018
@blankdots blankdots deleted the feature/flake8-docstrings branch September 20, 2018 04:32
viklund pushed a commit that referenced this pull request Nov 22, 2018
Make everything validate flake8-docstrings
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants