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

set correct content type for zapp files #155

Merged
merged 1 commit into from
Jul 14, 2014
Merged

Conversation

pkit
Copy link
Member

@pkit pkit commented Jul 10, 2014

*.zapp file is application/x-tar
now it will be set correctly on deploy
will also force application/json for job description json file

@pkit
Copy link
Member Author

pkit commented Jul 11, 2014

On my machine tox tests now fail completely in some strange places. I have a feeling that test-requirements do not include some of them. Is jinja2 needed?

@larsbutler
Copy link
Member

Yes. If you haven't already, try re-creating your test environment (with
tox -r).

On Fri, Jul 11, 2014 at 10:15 AM, Constantine Peresypkin <
notifications@github.com> wrote:

On my machine tox tests now fail completely in some strange places. I have
a feeling that test-requirements do not include some of them. Is jinja2
needed?


Reply to this email directly or view it on GitHub
#155 (comment).

@larsbutler
Copy link
Member

Some of the tests needs to be updated. Here's a patch which does that, and also updates the docstring of _generate_uploads to reflect the new return value.

--- a/zpmlib/tests/test_zpm.py
+++ b/zpmlib/tests/test_zpm.py
@@ -429,11 +429,13 @@ print("Hello from ZeroVM!")
         foojs = foojs_tmpl.render(auth_opts=self.auth_opts)

         expected_uploads = [
-            ('%s/zapp.yaml' % self.target, gzip.open(self.zapp_path).read()),
+            ('%s/zapp.yaml' % self.target, gzip.open(self.zapp_path).read(),
+             'application/x-tar'),
             ('%s/hello.json' % self.target,
-             self.job_json_prepped.decode('utf-8')),
-            ('%s/foo.js' % self.target, foojs),
-            ('%s/index.html' % self.target, self.indexhtml_contents),
+             self.job_json_prepped.decode('utf-8'),
+             'application/json'),
+            ('%s/foo.js' % self.target, foojs, None),
+            ('%s/index.html' % self.target, self.indexhtml_contents, None),
         ]
         assert uploads[0] == expected_uploads[0]
         assert uploads[1][0] == expected_uploads[1][0]
@@ -443,14 +445,16 @@ print("Hello from ZeroVM!")

     def test__deploy_zapp(self):
         with mock.patch('zpmlib.zpm._generate_uploads') as pu:
-            pu.return_value = iter([('x/a', 'b'), ('x/c', 'd')])
+            pu.return_value = iter([('x/a', 'b', None), ('x/c', 'd', None)])
             zpm._deploy_zapp(self.conn, self.target, self.zapp_path,
                              self.auth_opts)

             put_object = self.conn.put_object
             assert put_object.call_count == 2
-            assert put_object.call_args_list == [mock.call('x', 'a', 'b'),
-                                                 mock.call('x', 'c', 'd')]
+            assert put_object.call_args_list == [
+                mock.call('x', 'a', 'b', content_type=None),
+                mock.call('x', 'c', 'd', content_type=None),
+            ]

     def test_deploy_project_execute(self):
         parser = commands.set_up_arg_parser()
@@ -524,13 +528,13 @@ def test__prepare_auth_v2():

 def test_deploy_with_index_html():
     with mock.patch('zpmlib.zpm._generate_uploads') as gu:
-        gu.return_value = iter([('cont/dir/index.html', 'data')])
+        gu.return_value = iter([('cont/dir/index.html', 'data', 'text/html')])
         index = zpm._deploy_zapp(mock.Mock(), 'cont', None, None)
         assert index == 'cont/dir/index.html'


 def test_deploy_without_index_html():
     with mock.patch('zpmlib.zpm._generate_uploads') as gu:
-        gu.return_value = iter([('cont/foo.html', 'data')])
+        gu.return_value = iter([('cont/foo.html', 'data', 'text/html')])
         index = zpm._deploy_zapp(mock.Mock(), 'cont', None, None)
         assert index == 'cont/'
diff --git a/zpmlib/zpm.py b/zpmlib/zpm.py
index e8fbd80..54ae457 100644
--- a/zpmlib/zpm.py
+++ b/zpmlib/zpm.py
@@ -420,8 +420,11 @@ def _deploy_zapp(conn, target, zapp_path, auth_opts):


 def _generate_uploads(conn, target, zapp_path, auth_opts):
-    """Generate sequence of (container-and-file-path, data) tuples."""
-    # returns a list of pairs: (container-and-file-path, data)
+    """
+    Generate sequence of (container-and-file-path, data, content-type)
+    tuples.
+    """
+    # returns a list of triples: (container-and-file-path, data, content-type)
     tar = tarfile.open(zapp_path, 'r:gz')
     zapp_config = yaml.safe_load(tar.extractfile('zapp.yaml'))

@larsbutler
Copy link
Member

Looks like it needs a rebase now.

@pkit
Copy link
Member Author

pkit commented Jul 14, 2014

      _, objects = conn.get_container(base_container)

E TypeError: 'Mock' object is not iterable

Looks like mock installation and other things are seriously screwed.
Did rm -fr .tox it did not help, still get strange cram and strange mock (obviously not the same version as working ones).

@larsbutler
Copy link
Member

Hmm, must be something dirty in your test directory. You could try cleaning everything: git clean -fdx and then do tox -r. If you don't have any luck with that, you can pastebin the full test output and I'll try to figure out what's wrong.

@pkit
Copy link
Member Author

pkit commented Jul 14, 2014

It borks only on a first run. Running tox second time worked OK.

*.zapp file is `application/x-tar`
now it will be set correctly on `deploy`
will also force `application/json` for job description json file
@pkit
Copy link
Member Author

pkit commented Jul 14, 2014

Totally reproducible. Delete .tox, run tox - dies. Rerun tox - no problem.

larsbutler added a commit that referenced this pull request Jul 14, 2014
set correct content type for zapp files
@larsbutler larsbutler merged commit abf49f5 into zerovm:master Jul 14, 2014
@pkit pkit deleted the content_type branch July 14, 2014 17:05
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