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

Fixes mostly to pip install #69

Merged
merged 1 commit into from
Feb 18, 2018

Conversation

JordiBForgeFlow
Copy link
Member

I am adding some fixes as I am getting more on hands with it. Specifically to the pip install.

But I also found that we need a default path for logging of odoo. Otherwise one can forget to add it to the playbook and the logs are then lost.

@@ -71,7 +71,7 @@ odoo_config_limit_time_real_cron: -1 # >= 10.0
odoo_config_list_db: True
odoo_config_log_db: False
odoo_config_log_level: info
odoo_config_logfile: None
odoo_config_logfile: "/var/log/{{ odoo_user }}/{{ odoo_user }}.log"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about this:

  1. The init script (and systemd unit) already configure the logfile option (https://github.com/OCA/ansible-odoo/blob/master/templates/odoo-pip.init#L23 , and they are using odoo_logdir and odoo_service to be compatible with multi odoo instances on one server).
  2. If this option stay undefined, it allows to run the server manually (dev/debug/update purpose) with the logs on the standard output

- name: Create odoo data dir directory
file: path={{ odoo_config_data_dir }} state=directory
owner={{ odoo_user }} group={{ odoo_user }} force=yes

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Odoo is not able to handle this case by its own?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it fails because the dir does not exist.

@@ -6,7 +6,7 @@ After=network.target
Type=simple
User={{ odoo_user }}
WorkingDirectory={{ odoo_workdir }}
ExecStart={{ odoo_pip_odoo_bin_path }}
ExecStart={{ odoo_pip_odoo_bin_path }} --config {{ odoo_config_file }}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 And the other odoo-standard.service file need the same fix.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

@@ -40,7 +41,7 @@ function _start() {
# from the configuration file
# Odoo: https://github.com/odoo/odoo/pull/13685
# OCB: https://github.com/OCA/OCB/pull/553
start-stop-daemon --chdir=${WORKDIR} --start --quiet --pidfile $PIDFILE --chuid $USER:$USER --background --make-pidfile --exec $DAEMON -- --logfile $LOGFILE{{ odoo_config_server_wide_modules not in [False, 'None', ''] and ' --load=%s' % odoo_config_server_wide_modules or '' }}
start-stop-daemon --chdir=${WORKDIR} --start --quiet --pidfile $PIDFILE --chuid $USER:$USER --background --make-pidfile --exec $DAEMON --config $CONFIG --logfile $LOGFILE{{ odoo_config_server_wide_modules not in [False, 'None', ''] and ' --load=%s' % odoo_config_server_wide_modules or '' }}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You removed the double-dash -- just before --config. It is mandatory to separate start-stop-daemon options from Odoo ones.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed!

- increases the default values for limit_memory_soft and limit_memory_hard,
as the previous were obsolete, applicable to openerp 7.0
@JordiBForgeFlow JordiBForgeFlow force-pushed the master-odoo_config_data_dir branch from 56c6835 to 0c8e963 Compare February 18, 2018 00:08
@JordiBForgeFlow
Copy link
Member Author

Providing wrong memory defaults can ruin your weekend! The PR also introduces fixes to #72

@pedrobaeza pedrobaeza merged commit 0de67c5 into OCA:master Feb 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants