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

Forward port config file setup from connector #5

Merged

Conversation

leorochael
Copy link

Forward port of OCA/connector#213.

Copy link
Owner

@guewen guewen left a comment

Choose a reason for hiding this comment

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

Thanks for porting this feature that I obviously missed during the migration (it has been a few months since I started the work and now). Could you just give your opinion on the name of the options-queue_job section? I'm not quite sure of what I prefer :)


.. code-block:: ini

[options-queue_job]
Copy link
Owner

Choose a reason for hiding this comment

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

I wonder if mixing - and _ wouldn't bring some confusion for users. I understand that queue_job represents the name of the addon, but maybe options-queue-job would be simpler? What do you think?

Copy link
Author

Choose a reason for hiding this comment

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

So, I originally made the section be called [options-connector] because I thought anybox.recipe.odoo could only make sections that were called [options-something].

Today, while pondering your question, I decided to see how hard it would be to change anybox to create any kind of section name, and then I realised I was mistaken, and actually it's possible to create any section name.

So I just rewrote the section name to simply [queue_job], while keeping backward compatibility with the old connector section name.

@leorochael leorochael force-pushed the 10.0-add-queue_job-config-file branch from 6d01367 to 13033cf Compare January 2, 2017 23:33
In addition to the ODOO_CONNECTOR_CHANNELS environment variable, for
backward compatibility, we can configure the runner channels through
the ODOO_QUEUE_JOB_CHANNELS and in the odoo configuration file like this:

    [options-job_queue]
    channels = root:4

For backward compatibility, the section `[options-connector]` is also
accepted.

Moreover, it's now also possible to preload the module in the config
file, like:

    [options]
    server_wide_modules = web,web_kanban,queue_job

As such we can configure the job runner using only the Odoo config file.
Ignore whitespace around values, and tolerate missing entries that would
be caused by trailing commas or commented lines when the channel
configuration is provided through the Odoo configuration file.

Also, accept line breaks channel entry separators along with commas,
which make the configuration file more readable.
@leorochael leorochael force-pushed the 10.0-add-queue_job-config-file branch from 13033cf to 25315f0 Compare January 2, 2017 23:38
@leorochael
Copy link
Author

Speaking of backward compatibility, There's a reference left to an ODOO_CONNECTOR_PORT environment variable.

Would you like it to be changed into ODOO_QUEUE_JOB_PORT as well?

And finally, should we keep backward compatibility with these old connector settings?

@guewen
Copy link
Owner

guewen commented Jan 3, 2017

[queue_job] is nice!

I don't mind if we forsake the backward compatibility as we change the Odoo version and many things around. But if you think it eases the transition to keep the connector options especially regarding the buildout recipes, I have nothing against keeping them.

@guewen
Copy link
Owner

guewen commented Jan 3, 2017

Would you like it to be changed into ODOO_QUEUE_JOB_PORT as well?

It would be great :)

Or replace them with references to queue_job.

Also, rip off the barely useful modicum of backward compatibility.
@leorochael
Copy link
Author

leorochael commented Jan 3, 2017

I don't mind if we forsake the backward compatibility as we change the Odoo version and many things around.

@guewen, I agree. configurations will have to be changed, at the very least, to load the correct module (not to mention the fact that model names changed, and a migration would need to be very well thought out anyway), so I don't see much value in the feeble attempts at backward compatibility we have so far.

So I added a commit removing all references and explicit backward compatibility with connector.

@guewen
Copy link
Owner

guewen commented Jan 4, 2017

Very nice, thanks

@guewen guewen merged commit e771761 into guewen:10.0-add-queue_job Jan 4, 2017
@leorochael
Copy link
Author

No problem! Thanks for the merge!

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