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

Support Django 1.8(.3) #167

Merged
merged 23 commits into from
Oct 18, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
0758d03
Django Upgrade v1.8.3
z3r0w0n Aug 20, 2015
e9ef43b
v1.8.3 - Some more best practices and bug fixes
z3r0w0n Aug 20, 2015
d328187
Django 1.7+ compatibility: mimetype -> content_type
awwad Nov 12, 2015
4e9c826
reverting import changes
awwad Nov 12, 2015
2520408
Django 1.8 compatibility fix for forms.py (also doc link update)
awwad Dec 21, 2015
8a41d85
Django 1.8 compatibility - removing enter_transaction_management call
awwad Jan 7, 2016
897e5f2
Django 1.7+ compatibility for backend, attempt, workaround for repy b…
awwad Jan 8, 2016
a00c45c
Django 1.7+ compatibility fix - django.setup in correct spot in stand…
awwad Jan 11, 2016
cc463bb
Removing migrations. Some are Sensibility-specific, rest are unnecess…
awwad Jan 11, 2016
fcafb75
django 1.7+ compat: adding missed django.setup to polling/check_activ…
awwad Jan 29, 2016
af7adc7
Updating text of installer download page to reflect Python requirements
awwad Feb 1, 2016
0ef5014
cleanup of <~> marks and line length for PR
awwad Mar 11, 2016
c849e07
Remove `enter_transaction_management()`
aaaaalbert Oct 12, 2016
e85f995
Restore Python `type` before `django.setup`
aaaaalbert Oct 12, 2016
df9e470
Remove deprecated `@transaction.commit_manually`
aaaaalbert Oct 12, 2016
4f3d50a
Call into `django.setup` directly.
aaaaalbert Oct 12, 2016
a0b117a
Remove Django 1.1 compatibility code
aaaaalbert Oct 12, 2016
56af630
Remove comment that should be a commit message
aaaaalbert Oct 12, 2016
d428418
Remove another commit-message-as-a-comment
aaaaalbert Oct 12, 2016
1695579
Remove a bit of trailing whitespace
aaaaalbert Oct 12, 2016
101289d
Re-add length restriction for node port number
aaaaalbert Oct 17, 2016
64257db
Revert changes to `geni_base.html` template
aaaaalbert Oct 18, 2016
3b3488e
Add newline to end of file in website/urls.py
aaaaalbert Oct 18, 2016
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 24 additions & 3 deletions backend/backend_daemon.py
Original file line number Diff line number Diff line change
Expand Up @@ -324,10 +324,29 @@ def JoinVessels(*args):

# Raises NodemanagerCommunicationError if it fails.
return nodemanager.join_vessels(nodehandle, firstvesselname, secondvesselname)




def run_django_setup_using_python_type():
"""
Helper function to run `django.setup` with `__builtins__.type`
set to the default Python `type` function (rather than Repy's).

We need to do this because other code could have imported
repy.py and/or safe.py and by this have overridden the built-in
`type` function. We need to undo this override temporarily
before running `django.setup()`, and restore it afterwards.
(Restoration makes sure that any following Repy code/libraries
see the overridden `type` again.)
"""
repy_safe_type = __builtins__.type
# safe.py stores a reference to Python's original `type`
import safe
__builtins__.type = safe._type
django.setup()
__builtins__.type = repy_safe_type



def cleanup_vessels():
"""
Expand All @@ -337,8 +356,8 @@ def cleanup_vessels():

log.info("[cleanup_vessels] cleanup thread started.")

# Start a transaction management.
django.db.transaction.enter_transaction_management()
run_django_setup_using_python_type()


# Run forever.
while True:
Expand Down Expand Up @@ -491,6 +510,8 @@ def sync_user_keys_of_vessels():

log.info("[sync_user_keys_of_vessels] thread started.")

run_django_setup_using_python_type()

Copy link
Contributor

Choose a reason for hiding this comment

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

DRY! Maybe we could put this mysterious piece of code into a function. See comment above for why it's mysterious.

Copy link
Contributor

Choose a reason for hiding this comment

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

See above --- we require a suitable Django version with django.setup from now on.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also addressed in e85f995, see above for caveats.

# Run forever.
while True:

Expand Down
52 changes: 25 additions & 27 deletions common/api/maindb.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,7 @@
We try to keep all manual transaction management in clearinghouse to within
this module. The general idea is that the default behavior of django is
what we want in most places (to commit any time data-altering functions
are called, such as .save() or .delete()). However, in a few cases we
want multiple data-altering functions to be committed atomically, so we
use @transaction.commit_manually.
are called, such as .save() or .delete()).
"""

# It is a bit confusing to just import datetime because then you have to use
Expand Down Expand Up @@ -159,7 +157,6 @@ def init_maindb():



@transaction.commit_manually
@log_function_call_and_only_first_argument
def create_user(username, password, email, affiliation, user_pubkey, user_privkey, donor_pubkey):
"""
Expand Down Expand Up @@ -219,21 +216,22 @@ def create_user(username, password, email, affiliation, user_pubkey, user_privke
# We're committing manually to make sure the multiple database writes are
# atomic. (That is, regenerate_api_key() will do a database write.)
try:
# Generate a random port for the user's usable vessel port.
port = random.sample(ALLOWED_USER_PORTS, 1)[0]

# Create the GeniUser (this is actually records in two different tables
# underneath because of model inheretance, but django hides that from us).
geniuser = GeniUser(username=username, password='', email=email,
affiliation=affiliation, user_pubkey=user_pubkey,
user_privkey=user_privkey, donor_pubkey=donor_pubkey,
usable_vessel_port=port,
free_vessel_credits=DEFAULT_FREE_VESSEL_CREDITS)
# Set the password using this function so that it gets hashed by django.
geniuser.set_password(password)
geniuser.save()

regenerate_api_key(geniuser)
with transaction.atomic():
# Generate a random port for the user's usable vessel port.
port = random.sample(ALLOWED_USER_PORTS, 1)[0]

# Create the GeniUser (this is actually records in two different tables
# underneath because of model inheretance, but django hides that from us).
geniuser = GeniUser(username=username, password='', email=email,
affiliation=affiliation, user_pubkey=user_pubkey,
user_privkey=user_privkey, donor_pubkey=donor_pubkey,
usable_vessel_port=port,
free_vessel_credits=DEFAULT_FREE_VESSEL_CREDITS)
# Set the password using this function so that it gets hashed by django.
geniuser.set_password(password)
geniuser.save()

regenerate_api_key(geniuser)
Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM, transaction.atomic() is the way to go, and when catching Exceptions insided the function the used context manager is preferred over a decorator.


except:
Copy link
Contributor

Choose a reason for hiding this comment

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

Btw. our code-guidelines urge to use Class-based exceptions (I know this is not part of this PR).

Copy link
Contributor

Choose a reason for hiding this comment

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

(Well-spotted! I've added issue #175 for this.)

transaction.rollback()
Expand Down Expand Up @@ -575,7 +573,6 @@ def create_vessel(node, vesselname):



@transaction.commit_manually
@log_function_call
def set_vessel_ports(vessel, port_list):
"""
Expand Down Expand Up @@ -604,13 +601,14 @@ def set_vessel_ports(vessel, port_list):
# We're committing manually to make sure the multiple database writes are
# atomic.
try:
# Delete all existing VesselPort records for this vessel.
VesselPort.objects.filter(vessel=vessel).delete()

# Create a VesselPort record for each port in port_list.
for port in port_list:
vesselport = VesselPort(vessel=vessel, port=port)
vesselport.save()
with transaction.atomic():
# Delete all existing VesselPort records for this vessel.
VesselPort.objects.filter(vessel=vessel).delete()

# Create a VesselPort record for each port in port_list.
for port in port_list:
vesselport = VesselPort(vessel=vessel, port=port)
vesselport.save()
Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM, see comment above.


except:
transaction.rollback()
Expand Down
12 changes: 12 additions & 0 deletions node_state_transitions/node_transition_lib.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,18 @@

from clearinghouse.website import settings

# Beginning of part of fix for Issue #152
# Perform django setup (required in Django 1.7+ (possibly 1.6.?+)) to make
# db/models available. We need to do this if we are using a django version
# that has django.setup() (1.7+?) This should happen before the repy patching
# of builtins, which likely occurs in the add_dy_support call in the next line
# of code. django.setup() expects to be able to use the builtin keyword
# 'type', which is patched by repy, so we should do this here, before that
# patching.
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider grooming the comment:

# In Django 1.7+ standalone scripts require a call to django.setup() before using django components
# This should happen before seattle patches python builtins like `type`.
# https://docs.djangoproject.com/en/1.8/topics/settings/#calling-django-setup-is-required-for-standalone-django-usage

Copy link
Contributor

Choose a reason for hiding this comment

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

See above --- don't provide backwards compatibility.

Copy link
Contributor

Choose a reason for hiding this comment

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

In contrast to polling/check_active_db_nodes.py, this here node_state_transitions/node_transition_lib.py does import repyportability which indirectly overrides type. However, again no attempts are made to undo the override. (I thus wonder if it's really necessary to restore Python's type before running django.setup()).

if hasattr(django, 'setup'):
django.setup()
# end of Issue #152 fix

# Import all the repy files.
add_dy_support(locals())

Expand Down
3 changes: 3 additions & 0 deletions polling/check_active_db_nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ def main():

lockserver_handle = lockserver.create_lockserver_handle()

django.setup()


# Always try to release the lockserver handle, though it's probably not
# very useful in this case.
try:
Expand Down
19 changes: 11 additions & 8 deletions website/control/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

<Author>
Justin Samuel
Sai Kaushik Borra

<Purpose>
This module provides classes that tell django how to represent the models
Expand All @@ -17,14 +18,15 @@

from clearinghouse.common.api import maindb

from clearinghouse.website.control.models import Donation
from clearinghouse.website.control.models import GeniUser
from clearinghouse.website.control.models import Node
from clearinghouse.website.control.models import Vessel
from clearinghouse.website.control.models import VesselPort
from clearinghouse.website.control.models import VesselUserAccessMap
from clearinghouse.website.control.models import ActionLogEvent
from clearinghouse.website.control.models import ActionLogVesselDetails
from models import Donation
from models import GeniUser
from models import Node
from models import Vessel
from models import VesselPort
from models import VesselUserAccessMap
from models import ActionLogEvent
from models import ActionLogVesselDetails

Copy link
Contributor

Choose a reason for hiding this comment

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

Does not seem like a deprecation/removal related change but looks fine, because models is in the same directory.


from django.contrib import admin
from django.contrib.auth.forms import AdminPasswordChangeForm
Expand Down Expand Up @@ -244,3 +246,4 @@ class ActionLogVesselDetailsAdmin(admin.ModelAdmin):
admin.site.register(VesselUserAccessMap, VesselUserAccessMapAdmin)
admin.site.register(ActionLogEvent, ActionLogEventAdmin)
admin.site.register(ActionLogVesselDetails, ActionLogVesselDetailsAdmin)

Empty file.
19 changes: 6 additions & 13 deletions website/control/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

<Author>
Justin Samuel
Sai Kaushik Borra

<Purpose>
This file contains definitions of model classes for the main database
Expand All @@ -31,7 +32,6 @@




# First, we want to register a signal. This page recommends putting this code
# in models.py: http://docs.djangoproject.com/en/dev/topics/signals/

Expand All @@ -40,17 +40,10 @@ def _prepare_newly_created_db_connection(sender, **kwargs):
from clearinghouse.common.api import maindb
maindb.init_maindb()

# If this is a modern-enough version of django to support specifying a function
# to be called on database connection creation, then have it call init_maindb()
# at that time. This is to help prevent init_maindb() from accidentally not
# being called when it should be.
if django.VERSION >= (1,1):
# connection_created only exists with django >= 1.1
import django.db.backends.signals
django.db.backends.signals.connection_created.connect(_prepare_newly_created_db_connection)
else:
log.error("You must use django >= 1.1 in order to support automatically " +
"perform custom database connection initialization. (See settings.py)")
# Call init_maindb() on database connection creation. This is to help prevent
# init_maindb() from accidentally not being called when it should be.
import django.db.backends.signals
django.db.backends.signals.connection_created.connect(_prepare_newly_created_db_connection)



Expand All @@ -60,7 +53,7 @@ def _prepare_newly_created_db_connection(sender, **kwargs):
class GeniUser(DjangoUser):
"""
Defines the GeniUser model. A GeniUser record represents a SeattleGeni user.

By extending the DjangoUser model, django will still create a separate table
in the database for the GeniUser model but will take care of making it look
the same to us.
Expand Down
14 changes: 12 additions & 2 deletions website/html/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,22 @@

Jason Chen
[email protected]

Sai Kaushik Borra
[email protected]
<Purpose>

<Usage>
For more information on forms in django see:
http://docs.djangoproject.com/en/dev/topics/forms/
"""


from clearinghouse.website.control.models import GeniUser

# from control.models import GeniUser, Sensor, SensorAttribute, ExperimentInfo, ExperimentSensor, ExperimentSensorAttribute


from django.contrib.auth.forms import UserCreationForm as DjangoUserCreationForm
import django.forms as forms

Expand All @@ -30,6 +38,7 @@
MAX_PUBKEY_UPLOAD_SIZE = 2048

class PubKeyField(forms.FileField):

def clean(self,value,initial):
forms.FileField.clean(self,value,initial)
if value is None:
Expand All @@ -46,6 +55,8 @@ def clean(self,value,initial):





class GeniUserCreationForm(DjangoUserCreationForm):

affiliation = forms.CharField(error_messages={'required': 'Enter an Affiliation'})
Expand Down Expand Up @@ -95,8 +106,7 @@ def clean_email(self):




def gen_edit_user_form(field_list=None, *args, **kwargs):
def gen_edit_user_form(field_list='__all__', *args, **kwargs):
"""
<Purpose>
Dynamically generates a EditUserForm depending on field_list.
Expand Down
2 changes: 2 additions & 0 deletions website/html/templates/accounts/accounts_base.html
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,9 @@
{% endblock nav_login %}

{% block nav_register %}

<td><a href="{% url 'register' %}">Register</a></td>

{% endblock nav_register %}

{% block nav_help %}
Expand Down
8 changes: 8 additions & 0 deletions website/html/templates/accounts/register.html
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@

Modified:
Gaetano Pressimone 2012


Sai Kaushik Borra 2015


<Purpose>
Django template file for {{ TESTBED }} {{ CLEARINGHOUSE }}'s 'register' page. The page
Expand All @@ -27,6 +31,7 @@

<Template Variables>
page_top_errors:

Error messages to display.

form:
Expand All @@ -36,6 +41,7 @@
.password2:
.affiliation:
.email:

.pubkey:
.gen_upload_choice:

Expand All @@ -54,6 +60,7 @@
{% endblock nav_register %}

{% block content %}

<div id="main">
<div id="middle">

Expand Down Expand Up @@ -125,3 +132,4 @@
</div>
</div>
{% endblock content %}

4 changes: 2 additions & 2 deletions website/html/templates/common/installers.html
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
<a href="{% url 'mac_installer' username %}">Download installer for Mac</a>
<p>
<strong>Instructions:</strong>
<br />(Note: You must have Python 2.5 installed)
<br />(Note: You must have Python 2.5, 2.6, or 2.7 installed)
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

</p>
<ol>
<li>Download the {{ TESTBED }} tarball to your computer</li>
Expand Down Expand Up @@ -61,7 +61,7 @@
<a href="{% url 'linux_installer' username %}">Download installer for Linux</a>
<p>
<strong>Instructions:</strong>
<br />(Note: You must have Python 2.5 installed)
<br />(Note: You must have Python 2.5, 2.6, or 2.7 installed)
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

</p>
<ol>
<li>Download the {{ TESTBED }} tarball to your computer</li>
Expand Down
Loading