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

Q. Populating a part of config param from environment variables #9

Open
mayankpahwa opened this issue Jan 4, 2018 · 11 comments
Open
Labels

Comments

@mayankpahwa
Copy link

I have a config paramter that stores the HDFS location of a file. The parameter has HDFS IP, port and the file location, something like hdfs://172.21.1.11:54310/schema/schema.txt.
Since IPs and ports are always changing, I do not want to hard code their values. I tried setting the HDFS IP and port as environment variables and passing them to the value of config param in the configuration file, ex: hdfs://$HDFS_IP:$HDFS_PORT/schema/schema.txt, but it didn't work.

I know this can be achieved at the code level by reading the IP and port from the environment, and concatenating them with the file location, but is there a way to do it at the configuration level?

@alexander-yakushev
Copy link
Contributor

alexander-yakushev commented Jan 10, 2018

It's possible to do this using the :delayed-transform option (see here). You'd do something like this in the config map:

{:hdfs-ip {:type :string}
 :hdfs-port {:type :number}
 :hdfs-url {:type :string
            :delayed-transform #(or % ;; If HDFS_URL was set explicitly, just use it.
                                    (format "hdfs://%s:%s/schema/schema.txt"
                                            (cfg/get :hdfs-ip) (cfg/get :hdfs-port)))}}

Then, the first time you do (cfg/get :hdfs-url) it will resolve into either the explicit value or the formatted string.

@mayankpahwa
Copy link
Author

Thanks a lot @alexander-yakushev for your response. I tried the above with multiple cases and below is what I have found

;; Configuration definition
(cfg/define
  {:hdfs-ip {:type :string}
   :hdfs-port {:type :number}
   :hdfs-url {:type :string
              :delayed-transform #(or %
                                      (str "hdfs://" (cfg/get :hdfs-ip) ":"
                                           (cfg/get :hdfs-port) "/schema/schema.txt"))}})
;; Main function
(defn -main
  "Omniconf Demo"
  [& args]
  (cfg/populate-from-cmd args)
  (cfg/verify :quit-on-error true)
  (prn (cfg/get :hdfs-url)))
  1. I ran lein run --hdfs-ip 192.168.1.124 --hdfs-port 54310 and got the following output
Omniconf configuration:
 {:hdfs-ip "192.168.1.124", :hdfs-port 54310}

nil

Couldn't understand why nil was printed instead of the concatenated string.

  1. Then I ran lein run --hdfs-ip 192.168.1.124 --hdfs-port 54321 --hdfs-url hdfs://192.168.1.124:54310/schema/schema.txt and got the output
Omniconf configuration:
 {:hdfs-ip "192.168.1.124",
 :hdfs-port 54310,
 :hdfs-url #<Delay@294a6b8e: :not-delivered>}

"hdfs://192.168.1.124:54310/schema/schema.txt"

This works as expected

  1. After playing around for a while, I changed the configuration definition to
;; Configuration definition
(cfg/define
  {:hdfs-ip {:type :string}
   :hdfs-port {:type :number}
   :hdfs-url {:type :string
              :delayed-transform #(or
                                   (str "hdfs://" (cfg/get :hdfs-ip) ":"
                                        (cfg/get :hdfs-port) "/schema/schema.txt") %)}})

Then I ran lein run --hdfs-ip 192.168.1.124 --hdfs-port 54310 --hdfs-url and got the ouput

Omniconf configuration:
 {:hdfs-ip "192.168.1.124",
 :hdfs-port 54310,
 :hdfs-url #<Delay@294a6b8e: :not-delivered>}

"hdfs://192.168.1.124:54310/schema/schema.txt"

Couldn't understand why this works fine

@alexander-yakushev
Copy link
Contributor

This is a bug, sorry. :delayed-transform was originally intended for values that you do provide but want to do some post-processing on them. I thought it also supported what I've described, but I was wrong.

I'll try to think of a fix soon. Meanwhile, you can set a default value to :hdfs-url. The working example:

(cfg/define
  {:hdfs-ip {:type :string}
   :hdfs-port {:type :number}
   :hdfs-url {:type :string
              :default nil ;; Needed for delayed transform to work
              :delayed-transform #(or %
                                      (str "hdfs://" (cfg/get :hdfs-ip) ":"
                                           (cfg/get :hdfs-port) "/schema/schema.txt"))}})

lein run --hdfs-ip 192.168.1.124 --hdfs-port 54310

@mayankpahwa
Copy link
Author

I am trying with the following

(cfg/define
  {:hdfs-ip {:type :string}
   :hdfs-port {:type :number}
   :hdfs-url {:type :string
              :default nil ;; Needed for delayed transform to work
              :delayed-transform #(or %
                                      (str "hdfs://" (cfg/get :hdfs-ip) ":"
                                           (cfg/get :hdfs-port) "/schema/schema.txt"))}})

(defn -main
  "Omniconf Demo"
  [& args]
  (cfg/populate-from-cmd args)
  (cfg/verify :quit-on-error true)
  (prn (cfg/get :hdfs-url)))

And lein run --hdfs-ip 192.168.1.124 --hdfs-port 54310

Still getting

Omniconf configuration:
 {:hdfs-ip "192.168.1.124", :hdfs-port 54310}

nil

Am I doing something wrong?

@alexander-yakushev
Copy link
Contributor

alexander-yakushev commented Jan 24, 2018

Damn, I must have had unclean state when testing. This seems to work though:

(cfg/define
  {:hdfs-ip {:type :string}
   :hdfs-port {:type :number}
   :hdfs-url {:type :string
              :default ::none ;; Needed for delayed transform to work
              :delayed-transform #(if-not (= % ::none) ;; To allow setting the URL explicitly
                                    %
                                    (str "hdfs://" (cfg/get :hdfs-ip) ":"
                                         (cfg/get :hdfs-port) "/schema/schema.txt"))}})

@mayankpahwa
Copy link
Author

Thanks. Works fine now.

@mayankpahwa
Copy link
Author

I have another question

My configuration definition is:

(cfg/define
  {:hdfs-ip {:type :string}
   :hdfs-port {:type :number}
   :hdfs-url {:type :string
              :default "nil"
              :delayed-transform (fn [v] (str (cfg/get [:hdfs-ip]) ":"
                                              (cfg/get [:hdfs-port])))}
   :hdfs-url-nested {:nested
                     {:url {:type :string
                            :default "nil"
                            :delayed-transform (fn [v] (str (cfg/get [:hdfs-ip]) ":"
                                                            (cfg/get [:hdfs-port])))}}}})

Main function is:

(defn -main
  "Omniconf Demo"
  [& args]
  (cfg/populate-from-cmd args)
  (cfg/verify :quit-on-error true)
  (prn (cfg/get :hdfs-url))
  (prn (cfg/get :hdfs-url-nested)))

And I run lein run --hdfs-ip 192.168.1.1 --hdfs-port 9200

I am getting the output

Omniconf configuration:
{:hdfs-ip "192.168.1.1",
 :hdfs-port 9200,
 :hdfs-url #<Delay@6fb365ed: :not-delivered>,
 :hdfs-url-nested {:url "nil"}}

"192.168.1.1:9200"
{:url "nil"}

Looks like delayed-transform isn't working for nested params. Do you know of any workaround?

I am using version 0.2.7.

@alexander-yakushev
Copy link
Contributor

Sorry! That is another bug. I've just pushed a fix to it as 0.2.8-SNAPSHOT, please try :).

@mayankpahwa
Copy link
Author

Works fine, thanks :)

I just wanted to ask one more question. My configuration definition is present in a separate file rather than in the core.clj. In the file, I have the same configuration as mentioned above. For defining the configuration, I am doing something like
(cfg/define (read-string (slurp (io/resource "conf.def"))))

When I run lein run --hdfs-ip 192.168.1.1 --hdfs-port 54310, I get the error clojure.lang.PersistentList cannot be cast to clojure.lang.IFn

Looks like the function is not evaluated after reading from the file. Is there any way to solve this?

@alexander-yakushev
Copy link
Contributor

That's because you put a function definition into the file. When you do read-string it does not get compiled but treated as plain s-expressions. You may eval it, but I strongly advise against that – that's a security hole.

Honestly, at this point, I think that the delayed-transform approach caused you more harm than good. Perhaps, it would be easier just to do

(cfg/set :hdfs-url (str (cfg/get :hdfs-ip) ":" (cfg/get :hdfs-port))

before calling (cfg/verify).

@mayankpahwa
Copy link
Author

Makes sense. Thanks a lot :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants