Uploaded image for project: 'Bigtop'
  1. Bigtop
  2. BIGTOP-1803

Overide Vagrant configurations from environment variables

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Trivial
    • Resolution: Fixed
    • Affects Version/s: 0.8.0
    • Fix Version/s: 1.0.0
    • Component/s: None
    • Labels:
      None

      Description

      Something like this:

      $ REPO="http://hostname/yum/path/to/" vagrant up
      
      1. BIGTOP-1803.1.patch
        1 kB
        YoungWoo Kim
      2. BIGTOP-1803.2.patch
        2 kB
        YoungWoo Kim

        Activity

        Hide
        jayunit100 jay vyas added a comment -

        Great idea . lets implement this for sure

        Show
        jayunit100 jay vyas added a comment - Great idea . lets implement this for sure
        Hide
        warwithin YoungWoo Kim added a comment -

        BIGTOP-1803.1.patch

        • Override vagrant configs from env variables
        Show
        warwithin YoungWoo Kim added a comment - BIGTOP-1803.1.patch Override vagrant configs from env variables
        Hide
        evans_ye Evans Ye added a comment -

        Hey YoungWoo Kim the patch looks neat!
        Just one most things need to take care of: num_instances will be treated as string instead of integer, which cause a failure when looping it over in the following execution.
        We can have it fix like this:

         
        -    CONF[k] = ENV[k.upcase]
        +    if /\d+/.match(ENV[k.upcase])
        +      CONF[k] = Integer(ENV[k.upcase])
        +    else
        +      CONF[k] = ENV[k.upcase]
        +    end
        

        In addition, would you mind to add a little bit doc in this patch directly? One or two line explicitly demonstrate the right kind of usage would be enough I think.
        Thanks for the patch anyway

        Show
        evans_ye Evans Ye added a comment - Hey YoungWoo Kim the patch looks neat! Just one most things need to take care of: num_instances will be treated as string instead of integer, which cause a failure when looping it over in the following execution. We can have it fix like this: - CONF[k] = ENV[k.upcase] + if /\d+/.match(ENV[k.upcase]) + CONF[k] = Integer (ENV[k.upcase]) + else + CONF[k] = ENV[k.upcase] + end In addition, would you mind to add a little bit doc in this patch directly? One or two line explicitly demonstrate the right kind of usage would be enough I think. Thanks for the patch anyway
        Hide
        warwithin YoungWoo Kim added a comment -

        BIGTOP-1803.2.patch:

        • Addressed Evans Ye's comment.
        • Add a example to README
        Show
        warwithin YoungWoo Kim added a comment - BIGTOP-1803.2.patch : Addressed Evans Ye 's comment. Add a example to README
        Hide
        evans_ye Evans Ye added a comment -

        Tested and +1. Although we're close to the release, this improvement looks no hurt. Will commit later.

        Show
        evans_ye Evans Ye added a comment - Tested and +1. Although we're close to the release, this improvement looks no hurt. Will commit later.
        Hide
        evans_ye Evans Ye added a comment -

        Committed. Thanks for the improvement YoungWoo Kim!

        Show
        evans_ye Evans Ye added a comment - Committed. Thanks for the improvement YoungWoo Kim !

          People

          • Assignee:
            warwithin YoungWoo Kim
            Reporter:
            warwithin YoungWoo Kim
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development