search

chbrown / twttr

Twitter API client for Clojure supporting REST and Streaming endpoints
35 stars 11 forks source link

Prefix environment variables with "TWITTER_" #6

Open bzg opened 5 years ago

bzg commented 5 years ago

I find ACCESS_TOKEN may conflict with environment variables for other applications/services.

chbrown commented 5 years ago

While I generally appreciate namespacing, this is liable to break things, beginning with my Travis CI tests, and ending with any end-users (including me) who call (env->UserCredentials) or (env->AppCredentials) without arguments.

Perhaps I should have chosen different names when forking this library and bumping to v3, but I've run the gauntlet of using several different Twitter clients in multiple languages, and these names seemed to me to be the consensus (though by no means universal).

I'll think on it a bit longer, but I don't think I'll merge this PR, sorry! :/

(Looking at your changes, I see you realize that (env->*Credentials ...) optionally takes an argument, so this example is primarily for any other users having the same issue.)

You can achieve exactly what you want by replacing all instances in your codebase of:

(env->UserCredentials)  ; <- old way, uses default mapping without TWITTER_ prefixes

with:

; explicitly specify environment variables to use and how:
(env->UserCredentials {:consumer-key      "TWITTER_CONSUMER_KEY"
                       :consumer-secret   "TWITTER_CONSUMER_SECRET"
                       :user-token        "TWITTER_ACCESS_TOKEN"
                       :user-token-secret "TWITTER_ACCESS_TOKEN_SECRET"})

...and similarly for (env->AppCredentials):

(env->AppCredentials {:consumer-key      "TWITTER_CONSUMER_KEY"
                      :consumer-secret   "TWITTER_CONSUMER_SECRET"})
bzg commented 5 years ago

Hi @chbrown, absolutely no problem in not merging, this was just a suggestion!

The solution you point to is actually good enough for me. I hope there won't be any confusion resulting from using different environment variables names in different apps (with or without the TWITTER_ prefix).

Thanks again for taking the time to handle this.