Skip to content

Conversation

@Alexis-Jacob
Copy link
Contributor

close #64

@Alexis-Jacob Alexis-Jacob force-pushed the dev-app_id_should_be_optional branch 2 times, most recently from 1b21230 to 7becd4a Compare October 1, 2019 17:46
Copy link
Contributor

@maingoh maingoh left a comment

Choose a reason for hiding this comment

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

Can you remove this line

If it fails raise a `DeepomaticException`.

and update the README.md (maybe explain app_id is now optional and api_key must be provided with named parameter)

@Alexis-Jacob Alexis-Jacob force-pushed the dev-app_id_should_be_optional branch from 29a0cf6 to b5734b1 Compare October 2, 2019 13:38
@Alexis-Jacob Alexis-Jacob merged commit c5f37ce into master Oct 3, 2019
@Alexis-Jacob Alexis-Jacob deleted the dev-app_id_should_be_optional branch October 3, 2019 11:52
raise DeepomaticException("Please specify 'app_id' and 'api_key' either by passing those values to the client"
" or by defining the DEEPOMATIC_APP_ID and DEEPOMATIC_API_KEY environment variables.")
if api_key is None:
raise DeepomaticException("Please specify 'api_key' either by passing those values to the client"
Copy link
Contributor

Choose a reason for hiding this comment

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

s/those values/it/

Copy link
Contributor

Choose a reason for hiding this comment

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

done in #69

" or by defining the DEEPOMATIC_APP_ID and DEEPOMATIC_API_KEY environment variables.")
if api_key is None:
raise DeepomaticException("Please specify 'api_key' either by passing those values to the client"
" or by defining the DEEPOMATIC_API_KEY environment variables.")
Copy link
Contributor

Choose a reason for hiding this comment

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

s/variables/variable/

Copy link
Contributor

@maingoh maingoh Oct 3, 2019

Choose a reason for hiding this comment

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

done in #69

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

APP_ID should be optional

4 participants