Merge lp:~matthias-cramer/charms/precise/haproxy/appsession into lp:charms/haproxy

Proposed by Matthias Cramer
Status: Rejected
Rejected by: Tom Haddon
Proposed branch: lp:~matthias-cramer/charms/precise/haproxy/appsession
Merge into: lp:charms/haproxy
Diff against target: 81 lines (+30/-2) (has conflicts)
3 files modified
config.yaml (+4/-0)
hooks/hooks.py (+22/-2)
revision (+4/-0)
Text conflict in hooks/hooks.py
Text conflict in revision
To merge this branch: bzr merge lp:~matthias-cramer/charms/precise/haproxy/appsession
Reviewer Review Type Date Requested Status
Tom Haddon Needs Fixing
charmers Pending
Review via email: mp+148651@code.launchpad.net

Description of the change

Added possibility to add appsession parameter to the configfile

To post a comment you must log in.
Revision history for this message
Tom Haddon (mthaddon) wrote :

There seem to be two text conflicts in the merge proposal - can you take a look?

Also, I think this may be achievable within the existing charm just by appending "spip_session len 34 timeout 3h request-learn" to the "service_options" parameter. As an example, if you have this:

    services: |
              - service_name: haproxy_service
                service_host: "0.0.0.0"
                service_port: 80
                service_options: [option httpchk GET /, balance leastconn]
                server_options: check inter 2000 rise 2 fall 5 maxconn 32

You could simply change it to this:

    services: |
              - service_name: haproxy_service
                service_host: "0.0.0.0"
                service_port: 80
                service_options: [option httpchk GET /, balance leastconn, spip_session len 34 timeout 3h request-learn]
                server_options: check inter 2000 rise 2 fall 5 maxconn 32

Can you try that and see if it does what you're hoping to achieve?

review: Needs Fixing
Revision history for this message
Pierre Amadio (pierre-amadio) wrote :

I was able to deploy the vanilla haproxy charm with the following yaml file:

melmoth@x220:~/dev/spip-juju$ cat spip.yaml
haproxy:
     default_timeouts: "queue 7000, connect 7000, client 7000, server 7000"
     services: |
                - service_name: haproxy_service
                  service_host: "0.0.0.0"
                  service_port: 80
                  service_options: [balance leastconn , appsession spip_session len 34 timeout 3h request-learn]
                  server_options: maxconn 100
spip:
     spip-origin: 3.0.5

The haproxy.cfg configuration file ends up with the same content as what it would have been using the merge proposal and setting listen_appsession to 'spip_session len 34 timeout 3h request-learn'

Looks to me the merge request is not needed.

Revision history for this message
Tom Haddon (mthaddon) wrote :

> I was able to deploy the vanilla haproxy charm with the following yaml file:
>
> melmoth@x220:~/dev/spip-juju$ cat spip.yaml
> haproxy:
> default_timeouts: "queue 7000, connect 7000, client 7000, server 7000"
> services: |
> - service_name: haproxy_service
> service_host: "0.0.0.0"
> service_port: 80
> service_options: [balance leastconn , appsession
> spip_session len 34 timeout 3h request-learn]
> server_options: maxconn 100
> spip:
> spip-origin: 3.0.5
>
> The haproxy.cfg configuration file ends up with the same content as what it
> would have been using the merge proposal and setting listen_appsession to
> 'spip_session len 34 timeout 3h request-learn'
>
>
> Looks to me the merge request is not needed.

Marking as rejected per this comment

Unmerged revisions

65. By Matthias Cramer

added more description

64. By Matthias Cramer

Added config parameter listen_appsession to set appsession parameter

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'config.yaml'
--- config.yaml 2012-10-10 14:38:47 +0000
+++ config.yaml 2013-02-15 10:30:34 +0000
@@ -66,6 +66,10 @@
66 default: False66 default: False
67 type: boolean67 type: boolean
68 description: Enable monitoring68 description: Enable monitoring
69 listen_appsession:
70 default: ""
71 type: string
72 description: appsession config e.g. 'spip_session len 34 timeout 3h request-learn'
69 monitoring_port:73 monitoring_port:
70 default: 1000074 default: 10000
71 type: int75 type: int
7276
=== modified file 'hooks/hooks.py'
--- hooks/hooks.py 2013-02-14 21:35:47 +0000
+++ hooks/hooks.py 2013-02-15 10:30:34 +0000
@@ -269,7 +269,7 @@
269#------------------------------------------------------------------------------269#------------------------------------------------------------------------------
270def create_listen_stanza(service_name=None, service_ip=None,270def create_listen_stanza(service_name=None, service_ip=None,
271 service_port=None, service_options=None,271 service_port=None, service_options=None,
272 server_entries=None):272 server_entries=None, listen_appsession=None):
273 if service_name is None or service_ip is None or service_port is None:273 if service_name is None or service_ip is None or service_port is None:
274 return(None)274 return(None)
275 service_config = []275 service_config = []
@@ -278,6 +278,8 @@
278 if service_options is not None:278 if service_options is not None:
279 for service_option in service_options:279 for service_option in service_options:
280 service_config.append(" %s" % service_option.strip())280 service_config.append(" %s" % service_option.strip())
281 if listen_appsession is not None:
282 service_config.append(" appsession %s" % listen_appsession.strip())
281 if server_entries is not None and isinstance(server_entries, list):283 if server_entries is not None and isinstance(server_entries, list):
282 for (server_name, server_ip, server_port, server_options) \284 for (server_name, server_ip, server_port, server_options) \
283 in server_entries:285 in server_entries:
@@ -449,7 +451,25 @@
449#------------------------------------------------------------------------------451#------------------------------------------------------------------------------
450def create_services():452def create_services():
451 services_list = get_config_services()453 services_list = get_config_services()
454<<<<<<< TREE
452 services_dict = get_services_dict()455 services_dict = get_services_dict()
456=======
457 services_dict = {}
458 for service_item in services_list:
459 service_name = service_item['service_name']
460 service_host = service_item['service_host']
461 service_port = service_item['service_port']
462 service_options = service_item['service_options']
463 server_options = service_item['server_options']
464 services_dict[service_name] = {'service_name': service_name,
465 'service_host': service_host,
466 'service_port': service_port,
467 'service_options': service_options,
468 'server_options': server_options}
469
470 config_data = config_get()
471 listen_appsession = yaml.load(config_data['listen_appsession'])
472>>>>>>> MERGE-SOURCE
453473
454 # service definitions overwrites user specified haproxy file in474 # service definitions overwrites user specified haproxy file in
455 # a pseudo-template form475 # a pseudo-template form
@@ -505,7 +525,7 @@
505 services_dict[service]['service_host'],525 services_dict[service]['service_host'],
506 services_dict[service]['service_port'],526 services_dict[service]['service_port'],
507 services_dict[service]['service_options'],527 services_dict[service]['service_options'],
508 server_entries))528 server_entries,listen_appsession))
509529
510530
511#------------------------------------------------------------------------------531#------------------------------------------------------------------------------
512532
=== modified file 'revision'
--- revision 2013-02-14 21:35:47 +0000
+++ revision 2013-02-15 10:30:34 +0000
@@ -1,1 +1,5 @@
1<<<<<<< TREE
142242
3=======
432
5>>>>>>> MERGE-SOURCE

Subscribers

People subscribed via source and target branches