search

korymath / talk-generator

talk-generator is capable of generating coherent slide decks based on a single topic suggestion.
MIT License
124 stars 9 forks source link

unsplash required? might need credentials or feature flag to shut off #49

Closed h0h0h0 closed 5 years ago

h0h0h0 commented 5 years ago

Feel free to close if not an issue.

It looks like unsplash credentials are required at this point? If so, we can add some validation code to settings.py to ensure that the generator doesn't start if the credentials are not set in .env. Or we can just shut off unsplash if the credentials are not set.

twinters commented 5 years ago

Ah yes, very well spotted!

This makes me think: do we actually want to prevent the generator of working if it is missing credentials (since it will not be using its maximum potential), or if we should just log a warning if these login details are missing/incorrect (since it is quite robust in this regard and thus will still be able to use the generators that do not require log in details, and thus generate worse slide decks). The latter has the advantage of being easier to quickly set-up for people wanting to try out the repository, which was a goal @korymath recently mentioned.

What do you two think/prefer? Raising a exception, or just a warning?

korymath commented 5 years ago

I think that we should build each generator as a value add... so that means it should throw a warning. The more auths you have, the better content you can farm.

On Thu, 15 Aug 2019 at 10:50, Thomas Winters notifications@github.com wrote:

Ah yes, very well spotted!

This makes me think: do we actually want to prevent the generator of working if it is missing credentials (since it will not be using its maximum potential), or if we should just log a warning if these login details are missing/incorrect (since it is quite robust in this regard and thus will still be able to use the generators that do not require log in details, and thus generate worse slide decks). The latter has the advantage of being easier to quickly set-up for people wanting to try out the repository, which was a goal @korymath https://github.com/korymath recently mentioned.

What do you two think/prefer? Raising a exception, or just a warning?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/korymath/talk-generator/issues/49?email_source=notifications&email_token=AABLPM2IDNF7DNS4IRCBCG3QEWCO3A5CNFSM4ILVVEXKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4MLNMI#issuecomment-521713329, or mute the thread https://github.com/notifications/unsubscribe-auth/AABLPM6E23APAYWEUIVVQYDQEWCO3ANCNFSM4ILVVEXA .

--

Kory Mathewson https://korymathewson.com

twinters commented 5 years ago

Alright I'll update it using warnings then!

twinters commented 5 years ago

Added as of https://github.com/korymath/talk-generator/commit/fefaefbd617e7e7cd9f43ef526381488853b2a70