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

apispec.utils.load_yaml_from_docstring should let YAMLError propagate #135

Closed
djanderson opened this issue Jul 11, 2017 · 1 comment

Comments

@djanderson
Copy link
Contributor

commented Jul 11, 2017

  • I think the "---" prefix is enough to keep from accidentally parsing random docstrings as YAML
  • the YAML parser raises very useful errors
  • swallowing the YAML parser error silently is probably not doing the user a favor
>>> docstring_with_no_yaml = """Some function docstring.
... 
... Nothing to see here.
... """
>>> docstring_with_yaml = """Some function docstring.
... ---
... get:
...   responses:
...     200:
...       schema: $ref: '#/definitions/Schema'
... """
>>> from apispec.utils import *
>>> load_yaml_from_docstring(docstring_with_no_yaml)
>>> # ok
... load_yaml_from_docstring(docstring_with_yaml)
>>> # huh?
... import yaml
>>> yaml.load(dedent('\n'.join(docstring_with_yaml.split('\n')[1:])))
Traceback (most recent call last):
[...]
yaml.scanner.ScannerError: mapping values are not allowed here
  in "<unicode string>", line 5, column 19:
          schema: $ref: '#/definitions/Schema'
                      ^ 

I propose changing

def load_yaml_from_docstring(docstring):
    """Loads YAML from docstring."""
    split_lines = trim_docstring(docstring).split('\n')

    # Cut YAML from rest of docstring
    for index, line in enumerate(split_lines):
        line = line.strip()
        if line.startswith('---'):
            cut_from = index
            break
    else:
        return None

    yaml_string = "\n".join(split_lines[cut_from:])
    yaml_string = dedent(yaml_string)
    try:
        return yaml.load(yaml_string)
    except yaml.YAMLError:
        return None

to

def load_yaml_from_docstring(docstring):
    """Loads YAML from docstring."""
    split_lines = trim_docstring(docstring).split('\n')

    # Cut YAML from rest of docstring
    for index, line in enumerate(split_lines):
        line = line.strip()
        if line.startswith('---'):
            cut_from = index
            break
    else:
        return None

    yaml_string = "\n".join(split_lines[cut_from:])
    yaml_string = dedent(yaml_string)
    return yaml.load(yaml_string)
@sloria

This comment has been minimized.

Copy link
Member

commented Aug 15, 2017

I think you are correct @djanderson . I would certainly review and merge a PR for this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.