Skip to content

Authentication, Type/Value checks, Use of News API error messages, and addressed a Windows cmd error#11

Merged
mattlisiv merged 6 commits intomattlisiv:masterfrom
bantaisaiah:master
Apr 4, 2018
Merged

Authentication, Type/Value checks, Use of News API error messages, and addressed a Windows cmd error#11
mattlisiv merged 6 commits intomattlisiv:masterfrom
bantaisaiah:master

Conversation

@bantaisaiah
Copy link
Copy Markdown
Contributor

@bantaisaiah bantaisaiah commented Apr 3, 2018

  • I added Type/Value Error messages to the optional variables in all functions, checking an additional file that has all variables and base URLs.
  • Added error catching for the top_headlines function according to the defined constraints laid out on the News API website: https://newsapi.org/docs/endpoints/top-headlines

Note: you can't mix this param with the country or category params.

  • Changed the name of the to parameter to to_param in order to be consistent with from_param
  • Added format check, with appropriate error message, to the to_param and from_param values
  • Added value checks to ints related to page parameters
  • Added Error code message to requests made which is provided by News API under their Errors section: https://newsapi.org/docs/errors
  • Updated README.md with an error that is produced when attempting to print the .json() object to the command line, and how to circumvent it in a healthy way with attached information as to the reasons why

@mattlisiv
Copy link
Copy Markdown
Owner

Hi @banta-isaiah ,

Thanks for the update. I really like the way these type checks are handled with the library.

A few things I think need to be adjusted:

  1. You have a typo in a few error statements where the flag is 'raist' rather than 'raise'
  2. On line 211, it should be:
if language not in const.languages:

rather than containing a 'is' .

  1. For now, let's keep the parameter as 'to' rather than 'to_param' . I definitely think it is better naming convention, but I don't want to worry about breaking anyone's app with a version upgrade or updating the documentation on newsapi.org without a CHANGELOG in place.

Thanks!

fixed line 211 to read `if language not in const.language`
changed parameter `to_param` to `to`
@bantaisaiah
Copy link
Copy Markdown
Contributor Author

Done! Let me know if there is anything else you see an issue with

@mattlisiv
Copy link
Copy Markdown
Owner

@banta-isaiah, it looks like you still have the to_param set as an argument within the function (line 121) .

def get_everything(self, q=None, sources=None, domains=None, from_param=None, to_param=None, language=None

Once that is corrected, everything looks good!

@mattlisiv mattlisiv merged commit 106dcb8 into mattlisiv:master Apr 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants