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

Fix Django 1.6 support, XML-RPC handler, tar file dirs, doc link, etc. #14

Merged
merged 39 commits into from
May 20, 2016

Conversation

aaaaalbert
Copy link
Contributor

Fixes #5 (XML-RPC interface) and #9 (vesselinfo directory mishap).

See the commit messages for details.

aaaaalbert and others added 30 commits August 5, 2014 16:17
Omitting the build file list for now -- `build.py` doesn't completely
supplant `preparetest` yet as it lacks a way to copy stuff to
sub-directories of the build target dir...
This build file is needed to set up the RepyV2 runtime which the
custominstallerbuilder requires to run. We need to do that because
plain Python `import`'s from Repy libraries don't currently work.

Future contributors, please check if this workaround is still needed
every time progress is made on SeattleTestbed/portability#25,
SeattleTestbed/portability#26, SeattleTestbed/portability#27, or
SeattleTestbed/portability#28!
This makes `seash` in the Repy runtime runnable, and lets you
generate user keys more easily than via the clearinghouse website.
I don't know why `simplejson` was ever `import`'ed `as json`.
Now that it doesn't seem to be around anymore, let's make sure
we have *some* kind of JSON support.
The SeattleTestbed/clearinghouse and SeattleTestbed/custominstallerbuilder
Django projects are laid out somewhat differently (for reasons lost
in history). I'll try to make them more similar as I go through the
changes required for supporting Django 1.6.

In this case, this comes at the expense of forcing the user to set
up environment variables (`DJANGO_SETTINGS_MODULE`, `PYTHONPATH`)
rather than trying to configure them automatically (or so it seems
--- I'll find out when testing.) Now is not the time for me to do
porting *and* debug clever logic at the same time, so I'm going
for the simpler, non-autoconfiguring method.
Cf. SantiagoTorres@a23561d670ec028e5f7e93cf64042852c576e075.
Cf. SantiagoTorres@d5466218b2d216ff4cce809b64652fd07caa0da9 in
website/settings.py
See SantiagoTorres@e519a16b19fe15c8a40c1a320cfe4051d8df7755
and the Django 1.5 release notes for reference.
Oops, new_build assumes `.` is the repo root.
Streamlines key generation
We only need that script from dist, but it was a 200MB download before.
We should also add an explanation who this user is supposed to be (the clearinghouse user, not the custominstallerbuilder user), and better separate the roles in the future.
…encies

dist does still include its own requirement file lists we currently rely on. Needs more refactoring.
This follows the example of SeattleTestbed/custominstallerbuilder@da5351a
(with due thanks to @SantiagoTorres who provided the initial
implementation.)
The property `raw_post_data` of Django `request`s was renamed to `body`, see relevant Django docs at https://code.djangoproject.com/ticket/17323
Remove the prefix `./` from `config_dir_rel`. This keeps `tar` from creating *two* dirs in the Linux tarball (`seattle` and `.`) which could confuse users, cause them to extract only one, and find `vesselinfo` missing when running the installer.
Remove the prefix `./` from `config_dir_rel`. This keeps `tar` from creating *two* dirs in the Linux tarball (`seattle` and `.`) which could confuse users, cause them to extract only one, and find `vesselinfo` missing when running the installer.
The property `raw_post_data` of Django `request`s was renamed to `body`, see relevant Django docs at https://code.djangoproject.com/ticket/17323 .

(This mirrors fix 04239c9 which should have gone to this development branch, not `master`.)
See note at https://docs.djangoproject.com/en/1.5/ref/templates/builtins/#std:templatetag-url:
"""
Warning

Don’t forget to put quotes around the function path or pattern name!
Changed in Django 1.5: The first parameter used not to be quoted, which was inconsistent with other template tags. Since Django 1.5, it is evaluated according to the usual rules: it can be a quoted string or a variable that will be looked up in the context.
"""
@lukpueh
Copy link
Contributor

lukpueh commented May 17, 2016

I am on it. But I spent half a day trying to actually set up CIB and CH to test the patches, and was forced to deal with other issues:

#15
SeattleTestbed/docs#14
#10 (comment)

I think the process and the docs to setup CH and CIB could be dramatically simplified. But I don't no if this is a pressing concern.

@aaaaalbert aaaaalbert changed the title Fix XML-RPC handler, tar file dirs, doc link Fix Django 1.6 support, XML-RPC handler, tar file dirs, doc link, etc. May 18, 2016
@lukpueh
Copy link
Contributor

lukpueh commented May 18, 2016

This PR just grew dramatically because I merged albert's django16support branch into his master (this PR's merge candidate).

Nevertheless, we can still focus on the last couple of commits as initially addressed by this PR, because in production we already use albert's django16support branch at commit 8bfafc2 (see production server git ouputs below) which is 7 commits behind HEAD of django16support (we want to merge HEAD).
Also in the CIB installation docs we instruct to checkout django16support.

I suggest that we update the docs and maybe also the production server's remote urls as soon as this has been reviewed and merged.

custominstallerbuilder@sensibilityclearinghouse:~/custominstallerbuilder$ git remote -v
origin  https://github.com/aaaaalbert/custominstallerbuilder.git (fetch)
origin  https://github.com/aaaaalbert/custominstallerbuilder.git (push)
custominstallerbuilder@sensibilityclearinghouse:~/custominstallerbuilder$ git branch
* django16support
custominstallerbuilder@sensibilityclearinghouse:~/custominstallerbuilder$ git log

commit 8bfafc2c04cce2eaf5ae853629ef5bd1d8330188
Author: aaaaalbert <[email protected]>
Date:   Mon Apr 13 16:03:17 2015 +0200

    Fix SeattleTestbed/custominstallerbuilder#9

    Remove the prefix `./` from `config_dir_rel`. This keeps `tar` from creating

<p>Interface with this page using an XML-RPC client, or use the
<a href="{% url builder %}">interactive builder</a>.
Visit the Seattle project wiki for the
<a href="https://seattle.poly.edu/wiki/CustomInstallerBuilderApi">XML-RPC API</a>.</p>
</div>
{% endblock %}</div>
{% endblock %}
Copy link
Contributor

Choose a reason for hiding this comment

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

Yep. We only need one {% endblock %}.

@lukpueh
Copy link
Contributor

lukpueh commented May 18, 2016

Here is how I tested this PR:

  1. Installed CIB (c.f. CIB Installation)

  2. Configured ~/custominstallerbuilder/local/settings.py (c.f. Create and configure local settings)

  3. Created a keypair (cf. generatekeys.py)

  4. Did not adapt the build scripts. Instead cloned, initialized and built installer-packaging repo

    • Changed the variables in ~/installer-packaging/RUNNABLE/rebuild_base_installers.py like so:

      software_update_url = 'http://seattle.cs.washington.edu/couvb/updatesite/0.1/'
      public_key_file = '/root/cib.publickey'
      private_key_file = '/root/cib.privatekey'
      
      base_installer_directory = '/home/cib/custominstallerbuilder/html/static/installers/base/'
      base_installer_archive_dir = '/home/cib/custominstallerbuilder/html/static/installers/old_base_installers/'
      
      user = 'cib'

      Note: The CIB docs suggest to use http://blackbox.poly.edu/updatesite/ as software_update_url but it seemed to me that this url was outdated

  5. Created base_installer and base_installer_archive_dir directories

  6. Modified both ~/custominstallerbuilder/DEPENDENCIES/softwareupdater.py and ~/installer-packaging/RUNNABLE/seattle_repy/softwareupdater.py to match with the above generated publickey like explained in the docs (c.f. softwareupdater URL and Keys)

  7. Created base installers:

    $ python ~/installer-packaging/RUNNABLE/rebuild_base_installers.py <version as in nmmain.py>
  8. Configured apache

I don't think CIB and installer-packaging are supposed to be used like this. What if we would only clone installer-packaging repo when building CIB and use the installer-packaging for both, creating the base installers and also to create a repy_runtime for the cib django app.

@@ -56,6 +56,6 @@ def xmlrpc_handler(request):
xmlrpc_handler.register_instance(PublicFunctions())

response = HttpResponse(mimetype='application/xml')
response.write(xmlrpc_handler._marshaled_dispatch(request.raw_post_data))
response.write(xmlrpc_handler._marshaled_dispatch(request.body))
Copy link
Contributor

Choose a reason for hiding this comment

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

I could reproduce the error before this change. And can verify that it works after applying the fix. Also, since this is just a name refactor, documented in the Django docs, I would say this is good to go.

@aaaaalbert
Copy link
Contributor Author

@lukpueh, asking for clarification of your last comment. Are you saying that the whole PR is good, or are you referring to the last bit in views.py only?

@@ -3,6 +3,9 @@
{% block content %}
<div class="section">
<h2>XML-RPC Interface</h2>
<p>Interface with this page using an XML-RPC client, or use the <a href="{% url builder %}">interactive builder</a>. Visit the Seattle project wiki for the <a href="https://seattle.cs.washington.edu/wiki/CustomInstallerBuilderAPI">XML-RPC API</a>.</p>
<p>Interface with this page using an XML-RPC client, or use the
<a href="{% url "builder" %}">interactive builder</a>.
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems to work. It finds the intended url which leads to the intended view which renders the intended html.

@lukpueh
Copy link
Contributor

lukpueh commented May 20, 2016

Clarification:
I just moved my inline-comments from the outdated diffs to the actual diffs that are going to be merged with this PR. All commits that this PR addressed initially are now tested and commented on.

How I tested this - Part 2:

  1. Locally set up CIB (Ubuntu 15.10., Python 2.7.10, Django 1.8.3, @aaaaalbert's cib/master)
  2. Locally set up CH (Ubuntu 15.10., Python 2.7.10, Django 1.8.3, @awwad's ch/fix_django)
  3. Surfed to the parts of CIB/CH that invoked the code from the relevant commits, before and after the change.
  4. Wrote comments on GitHub

Conclusion:
As stated earlier, I only reviewed the most recent diffs, those that were originally addressed by this PR. All the earlier commits are already used in production and therefor belong to master anyways.
Let's merge this PR!

Caveats:

@aaaaalbert
Copy link
Contributor Author

OK, merging!

@aaaaalbert aaaaalbert merged commit 8750ffd into SeattleTestbed:master May 20, 2016
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.

2 participants