733478 - Move view imports to web/views/__init_

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Erick Dransch [:edransch]]

Attachment: Move imports to web/views/__init__.py

We should make a slight design improvement and move the imported views from auslib/web/base.py to auslib/web/veiws/__init.py . auslib/web/base.py should import auslib/web/views

Comment 1

[[e-mail] (:bhearsum)]

Comment on attachment 603372 [details] [diff] [review]
Move imports to web/views/__init__.py

Review of attachment 603372 [details] [diff] [review]:
-----------------------------------------------------------------

::: auslib/web/views/__init__.py
@@ +1,3 @@
> +#Import all of the views in this folder
> +from auslib.web.views.permissions import *
> +from auslib.web.views.releases import *

Hmmm, I was thinking about this a bit more, and I think we should provide __all__ indexes (if you haven't heard of them, you can read about them here: http://docs.python.org/tutorial/modules.html#importing-from-a-package) in the specific view files. That is, permissions.py & releases.py. By doing that we'll avoid importing anything unnecessary from them. I don't think we need to bother for __init__.py yet, since it will just be a combination of all the imports we want from both files.

r=me with that addition.

Comment 2

[Erick Dransch [:edransch]]

Attachment: Move imports to web/views/__init__.py

Add __all__ to views.

Comment 3

[[e-mail] (:bhearsum)]

Comment on attachment 605971 [details] [diff] [review]
Move imports to web/views/__init__.py

Review of attachment 605971 [details] [diff] [review]:
-----------------------------------------------------------------

Looks fine, passes tests. This applies on top of my latest patch from bug 702595 so we'll wait until that lands to land it.

Comment 4

[Erick Dransch [:edransch]]

This patch still applies cleanly.

Comment 5

[[e-mail] (:bhearsum)]

Comment on attachment 605971 [details] [diff] [review]
Move imports to web/views/__init__.py

I landed this patch along with bug 734474, and the first Jenkins run burned with:
======================================================================
FAIL: testAddRelease (auslib.test.test_db.TestReleases)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/var/lib/jenkins/jobs/Balrog/workspace/auslib/test/test_db.py", line 458, in testAddRelease
    self.assertEquals(self.releases.t.select().where(self.releases.name=='d').execute().fetchall(), expected)
AssertionError: [] != [('d', 'd', 'd', '{"name": 4}', 1)]
-------------------- >> begin captured logging << --------------------
auslib.db: DEBUG: AUSTable._prepareInsert: Executing query: 'INSERT INTO releases (name, product, version, data, data_version) VALUES (?, ?, ?, ?, ?)' with values: {'data_version': 1, 'product': 'd', 'version': 'd', 'data': '{"name": 4}', 'name': 'd'}
auslib.db: DEBUG: History.getTimestamp: returning 1332533253657
--------------------- >> end captured logging << ---------------------

I can't repro locally, or on Jenkins, so I left it. However, I'd like to dig into it some more...I don't like having random orange :(.

Comment 6

[[e-mail] (:bhearsum)]

Filed https://bugzilla.mozilla.org/show_bug.cgi?id=738773 to track the intermittent orange.