Bug 969696 - Downloadable cartridge settings are not configurable via broker.conf
Summary: Downloadable cartridge settings are not configurable via broker.conf
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: OpenShift Online
Classification: Red Hat
Component: Pod
Version: 2.x
Hardware: Unspecified
OS: Unspecified
unspecified
medium
Target Milestone: ---
: ---
Assignee: Rajat Chopra
QA Contact: libra bugs
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2013-06-01 22:23 UTC by Clayton Coleman
Modified: 2015-05-15 00:17 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2013-06-24 14:50:14 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)
development.rb (4.88 KB, application/x-ruby)
2013-06-19 13:01 UTC, Jianwei Hou
no flags Details

Description Clayton Coleman 2013-06-01 22:23:36 UTC
None of 

  config.downloaded_cartridges = {
    :max_downloaded_carts_per_app => 5,
    :max_download_redirects => 2,
    :max_cart_size => 20480,
    :max_download_time => 10
  }

is exposed via broker configuration in broker.conf.  It should be (in both Origin and Online).

Comment 1 Rajat Chopra 2013-06-18 03:47:23 UTC
online.commit_id: b30b755 
origin.commit_id: a41cebb

Comment 2 Jianwei Hou 2013-06-18 06:22:36 UTC
Tested on devenv_3375
The settings are configurable in broker.conf and broker-dev.conf

# Give users the ability to download a cartridge into their gears on creation and cartridge add
DOWNLOAD_CARTRIDGES_ENABLED="true"
# For downloading cartridges, follow these constraints
MAX_DOWNLOADED_CARTS_PER_APP="5"
MAX_DOWNLOAD_REDIRECTS="2"
MAX_DOWNLOAD_TIME="10"
# Maximum size for downloadable manifest file (in bytes)
MAX_CART_SIZE="20480"

However I couldn't find the code that read and parse the configurations from broker.conf. I didn't see the set of codes regarding 'config.downloaded_cartridges' in development.rb or production.rb on my current instance of devenv_3375. Looks like new changes to development.rb and production.rb are not packaged in.

Comment 3 Rajat Chopra 2013-06-18 16:57:17 UTC
Fixed now.

Comment 4 Jianwei Hou 2013-06-19 13:01:06 UTC
Created attachment 762949 [details]
development.rb

Still didn't find this part of code in development.rb and production.rb. Test server was devenv_3384. Attached development.rb

config.downloaded_cartridges = {
    :max_downloaded_carts_per_app => conf.get("MAX_DOWNLOADED_CARTS_PER_APP", "5").to_i,
    :max_download_redirects => conf.get("MAX_DOWNLOAD_REDIRECTS", "2").to_i,
    :max_cart_size => conf.get("MAX_CART_SIZE", "20480").to_i,
    :max_download_time => conf.get("MAX_DOWNLOAD_TIME", "10").to_i
  }

Comment 5 Rajat Chopra 2013-06-19 18:05:11 UTC
The development.rb (in comment#4) looks good to me. It does use the config variables. 
Anything I am missing?

Comment 6 Jianwei Hou 2013-06-20 02:25:29 UTC
(In reply to Rajat Chopra from comment #5)
> The development.rb (in comment#4) looks good to me. It does use the config
> variables. 
> Anything I am missing?

Hi, Rajat, the content I listed in comment 4 is not present in development.rb and production.rb, as a result, I doubt whether the broker is able to use the config variables. You can open the added attachment to find out they are missing, the development.rb is copied from devenv instance on EC2

Comment 7 Xiaoli Tian 2013-06-20 02:41:15 UTC
That also could be to say: the new content change for development.rb is not built in rhc-broker, that means yum upgrade may not pick up the change, not sure if it's intended, or we need to add them in release ticket to ask ops to add them manually?

Comment 8 Rajat Chopra 2013-06-20 05:19:35 UTC
I realise what happened.. the pull request failed to merge. And I thought it had merged, so I read it wrong. Sorry :).

As soon as this one gets in then: https://github.com/openshift/li/pull/1650
The queue is somehow stuck right now though - I have updated the request with the fixed code.

Comment 9 Rajat Chopra 2013-06-20 06:10:45 UTC
Its merged now.

Comment 10 Jianwei Hou 2013-06-21 02:35:02 UTC
Verified on devenv_3396, now PR is merged, and the code is in development.rb/production.rb

  config.downloaded_cartridges = {
    :max_downloaded_carts_per_app => conf.get("MAX_DOWNLOADED_CARTS_PER_APP", "5").to_i,
    :max_download_redirects => conf.get("MAX_DOWNLOAD_REDIRECTS", "2").to_i,
    :max_cart_size => conf.get("MAX_CART_SIZE", "20480").to_i,
    :max_download_time => conf.get("MAX_DOWNLOAD_TIME", "10").to_i
  }


Note You need to log in before you can comment on or make changes to this bug.