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

WIP: Jinja2 for Web server templates #5994

Closed
wants to merge 3 commits into from

Conversation

manics
Copy link
Member

@manics manics commented Apr 4, 2019

What this PR does

Replaces python string formatting with Jinja2 when generating web-server configurations. This also allows the three separate Nginx templates to be combined into one, replacing duplicated config stanzas by some Jinja2 conditionals.

In future this may allow the OMERO.web Ansible role to deterministically generate and update the Nginx config. Currently this requires some logic to guess when the config needs to be regenerated: https://github.com/openmicroscopy/ansible-role-omero-web/blob/2.0.1/tasks/web-nginx.yml#L12-L37
For example: currently if you make an OMERO config change after the nginx config has already been generated it won't be updated until you upgrade OMERO.web. (Note that Ansible does not support remote templates so it would require an extra step: https://www.reddit.com/r/ansible/comments/7r0eno/using_a_template_on_the_remote_host/dste442/)

TODO: Currently the templated variable names are unchanged, though if this is incorporated into the Ansible role they will need to be renamed.

This PR is on top of #5995 to make it easier to review.

Testing this PR

It should be enough to verify the unit tests pass with only whitespace changes to the reference templates as shown by this filtered view: https://github.com/openmicroscopy/openmicroscopy/pull/5994/files?file-filters%5B%5D=.conf&w=1

Related reading

@manics manics force-pushed the web-template-j2 branch from 7f1dfc5 to a862291 Compare April 5, 2019 12:14
@joshmoore joshmoore added exclude transfer Migrate to another repository labels Aug 20, 2019
manics added a commit to manics-archive/omero-web that referenced this pull request Nov 11, 2019
$ curl -sSL https://patch-diff.githubusercontent.com/raw/ome/openmicroscopy/pull/5994.diff | sed -e 's%/components/tools/OmeroPy/%/%g' -e 's%/components/tools/OmeroWeb/%/%g' -e 's%etc/templates/web/%omeroweb/templates/%g' -e 's%/test/unit/clitest/%/test/unit/%g' -e 's%/src/%/%g' | git apply --reject

Checking patch omero/plugins/web.py...
error: while searching for:

import traceback
from datetime import datetime
from omero.cli import DiagnosticsControl
from omero.cli import CLI
import platform

error: patch failed: omero/plugins/web.py:10
Hunk ome#2 succeeded at 276 (offset 1 line).
error: while searching for:
                         "Web template configuration requires"
                         "wsgi or wsgi-tcp.")

        template_file = "%s.conf.template" % server
        c = file(self._get_web_templates_dir() / template_file).read()
        self.ctx.out(c % d)

    def syncmedia(self, args):
        self.collectstatic()

error: patch failed: omero/plugins/web.py:301
Checking patch test/unit/reference_templates/nginx-development-withoptions.conf...
Checking patch test/unit/reference_templates/nginx-development.conf...
Checking patch test/unit/reference_templates/nginx-location-withoptions.conf...
Checking patch test/unit/reference_templates/nginx-location.conf...
Checking patch test/unit/reference_templates/nginx-withoptions.conf...
Checking patch test/unit/reference_templates/nginx.conf...
Checking patch omeroweb/templates/nginx-development.conf.template...
Checking patch omeroweb/templates/nginx-location.conf.template...
Checking patch omeroweb/templates/nginx.conf.j2...
Checking patch omeroweb/templates/nginx.conf.template...
Applying patch omero/plugins/web.py with 2 rejects...
Rejected hunk ome#1.
Hunk ome#2 applied cleanly.
Rejected hunk ome#3.
Applied patch test/unit/reference_templates/nginx-development-withoptions.conf cleanly.
Applied patch test/unit/reference_templates/nginx-development.conf cleanly.
Applied patch test/unit/reference_templates/nginx-location-withoptions.conf cleanly.
Applied patch test/unit/reference_templates/nginx-location.conf cleanly.
Applied patch test/unit/reference_templates/nginx-withoptions.conf cleanly.
Applied patch test/unit/reference_templates/nginx.conf cleanly.
Applied patch omeroweb/templates/nginx-development.conf.template cleanly.
Applied patch omeroweb/templates/nginx-location.conf.template cleanly.
Applied patch omeroweb/templates/nginx.conf.j2 cleanly.
Applied patch omeroweb/templates/nginx.conf.template cleanly.
@joshmoore
Copy link
Member

Closing in favor of ome/omero-web#63

@joshmoore joshmoore closed this Nov 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exclude transfer Migrate to another repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants