Major files refactoring - isolate.
- Split main from handlers declaration for endpoints handlers.
- Remove unnecessary imports.
- Remove APP from mapreduce_jobs, it unnecessary.
- Rename source files for consistency.
- Enforce debug parameter on all create_application().
- Remove appstats, the new Cloud Console Trace features can give meaningful data
without require intervention from within the app.
R=vadimsh@chromium.org
BUG=
Review URL: https://codereview.appspot.com/221150043
diff --git a/appengine/isolate/app.yaml b/appengine/isolate/app.yaml
index 7bf3fa6..d40689e 100644
--- a/appengine/isolate/app.yaml
+++ b/appengine/isolate/app.yaml
@@ -13,16 +13,16 @@
handlers:
- url: /_ah/spi/.*
- script: endpoint_handlers_api.app
+ script: main_frontend.endpoint_app
secure: always
- url: /restricted/mapreduce(/.*)?
- script: mapreduce_jobs.app
+ script: mapreduce.main.APP
secure: always
login: admin
- url: /restricted.*
- script: main_frontend.app
+ script: main_frontend.frontend_app
secure: always
login: required
@@ -44,11 +44,10 @@
X-Frame-Options: deny
- url: /.*
- script: main_frontend.app
+ script: main_frontend.frontend_app
secure: always
builtins:
-- appstats: on
- remote_api: on
inbound_services:
diff --git a/appengine/isolate/appengine_config.py b/appengine/isolate/appengine_config.py
index 4cdbb4c..a4ce9c8 100644
--- a/appengine/isolate/appengine_config.py
+++ b/appengine/isolate/appengine_config.py
@@ -2,10 +2,9 @@
# Use of this source code is governed by the Apache v2.0 license that can be
# found in the LICENSE file.
-"""Configures includes (appstats and components.auth).
+"""Configures includes (components.auth).
https://developers.google.com/appengine/docs/python/tools/appengineconfig
"""
-appstats_CALC_RPC_COSTS = False
components_auth_UI_APP_NAME = 'Isolate Server'
diff --git a/appengine/isolate/handlers_backend.py b/appengine/isolate/handlers_backend.py
index a7deb25..34c045e 100644
--- a/appengine/isolate/handlers_backend.py
+++ b/appengine/isolate/handlers_backend.py
@@ -479,7 +479,7 @@
]
-def create_application(debug=False):
+def create_application(debug):
"""Creates the url router for the backend.
The backend only implements urls under /internal/.
diff --git a/appengine/isolate/endpoint_handlers_api.py b/appengine/isolate/handlers_endpoints.py
similarity index 96%
rename from appengine/isolate/endpoint_handlers_api.py
rename to appengine/isolate/handlers_endpoints.py
index f749af4..860fc65 100644
--- a/appengine/isolate/endpoint_handlers_api.py
+++ b/appengine/isolate/handlers_endpoints.py
@@ -6,17 +6,14 @@
import binascii
import datetime
-import hashlib
-import hmac
-import httplib
-import os
import re
-import sys
import time
-import urllib
-BASE_DIR = os.path.dirname(os.path.abspath(__file__))
-sys.path.insert(0, os.path.join(BASE_DIR, 'third_party'))
+from google.appengine import runtime
+from google.appengine.api import datastore_errors
+from google.appengine.api import memcache
+from google.appengine.api import taskqueue
+from google.appengine.ext import ndb
import endpoints
from protorpc import message_types
@@ -24,22 +21,15 @@
from protorpc import remote
from components import auth
-from components import ereporter2
from components import utils
+
import config
import gcs
from handlers_api import hash_content
from handlers_api import MIN_SIZE_FOR_DIRECT_GS
-from handlers_api import MIN_SIZE_FOR_GS
import model
import stats
-from google.appengine import runtime
-from google.appengine.api import datastore_errors
-from google.appengine.api import memcache
-from google.appengine.api import taskqueue
-from google.appengine.ext import ndb
-
### Request Types
@@ -462,11 +452,3 @@
payload = ''.join(
binascii.unhexlify(digest.digest) for digest in collection.items)
return utils.enqueue_task(url, 'tag', payload=payload)
-
-
-def create_application():
- ereporter2.register_formatter()
- return endpoints.api_server([IsolateService])
-
-
-app = create_application()
diff --git a/appengine/isolate/endpoint_handlers_api_test.py b/appengine/isolate/handlers_endpoints_test.py
similarity index 85%
rename from appengine/isolate/endpoint_handlers_api_test.py
rename to appengine/isolate/handlers_endpoints_test.py
index ef560d3..eb56872 100755
--- a/appengine/isolate/endpoint_handlers_api_test.py
+++ b/appengine/isolate/handlers_endpoints_test.py
@@ -6,8 +6,6 @@
import base64
import json
import logging
-import os
-import re
import sys
import unittest
from Crypto.PublicKey import RSA
@@ -17,9 +15,7 @@
from google.appengine.api import memcache
from google.appengine.api import taskqueue
-from google.appengine.api import urlfetch
-import endpoints
from protorpc.remote import protojson
import webapp2
import webtest
@@ -29,11 +25,9 @@
from test_support import test_case
import config
-import endpoint_handlers_api
-from endpoint_handlers_api import DigestCollection
-from endpoint_handlers_api import UPLOAD_MESSAGES
import gcs
import handlers_backend
+import handlers_endpoints
import model
@@ -60,14 +54,14 @@
Returns:
a Digest corresponding to the content/ namespace pair
"""
- return endpoint_handlers_api.Digest(
+ return handlers_endpoints.Digest(
digest=hash_content(content, namespace), size=len(content))
def generate_collection(contents, namespace=None):
if namespace is None:
- namespace = endpoint_handlers_api.Namespace()
- return DigestCollection(
+ namespace = handlers_endpoints.Namespace()
+ return handlers_endpoints.DigestCollection(
namespace=namespace,
items=[generate_digest(content, namespace.namespace)
for content in contents])
@@ -89,15 +83,15 @@
ticket = preupload_status.get('upload_ticket')
url = preupload_status.get('gs_upload_url', None)
if url is not None:
- return endpoint_handlers_api.FinalizeRequest(upload_ticket=ticket)
- return endpoint_handlers_api.StorageRequest(
+ return handlers_endpoints.FinalizeRequest(upload_ticket=ticket)
+ return handlers_endpoints.StorageRequest(
upload_ticket=ticket, content=content)
-validate = endpoint_handlers_api.TokenSigner.validate
+validate = handlers_endpoints.TokenSigner.validate
-def pad_string(string, size=endpoint_handlers_api.MIN_SIZE_FOR_DIRECT_GS):
+def pad_string(string, size=handlers_endpoints.MIN_SIZE_FOR_DIRECT_GS):
return string + '0' * (size - len(string))
@@ -120,7 +114,7 @@
class IsolateServiceTest(test_case.EndpointsTestCase):
"""Test the IsolateService's API methods."""
- api_service_cls = endpoint_handlers_api.IsolateService
+ api_service_cls = handlers_endpoints.IsolateService
gs_prefix = 'localhost:80/content-gs/store/'
store_prefix = 'https://sample-app.storage.googleapis.com/'
@@ -135,6 +129,8 @@
self.mock(utils, 'get_task_queue_host', lambda: version)
self.testbed.setup_env(current_version_id='testbed.version')
self.source_ip = '127.0.0.1'
+ # It is needed solely for self.execute_tasks(), which processes tasks queues
+ # on the backend application.
self.app = webtest.TestApp(
webapp2.WSGIApplication(handlers_backend.get_routes(), debug=True),
extra_environ={'REMOTE_ADDR': self.source_ip})
@@ -169,8 +165,11 @@
'preupload', self.message_to_dict(good_digests), 200)
message = response.json.get(u'items', [{}])[0]
self.assertEqual('', message.get(u'gs_upload_url', ''))
+ expected = validate(
+ message.get(u'upload_ticket', ''),
+ handlers_endpoints.UPLOAD_MESSAGES[0])
self.assertEqual(
- validate(message.get(u'upload_ticket', ''), UPLOAD_MESSAGES[0]),
+ expected,
generate_embedded(good_digests.namespace, good_digests.items[0]))
def test_finalize_url_ok(self):
@@ -181,26 +180,29 @@
message = response.json.get(u'items', [{}])[0]
self.assertTrue(message.get(u'gs_upload_url', '').startswith(
self.store_prefix))
+ expected = validate(
+ message.get(u'upload_ticket', ''),
+ handlers_endpoints.UPLOAD_MESSAGES[1])
self.assertEqual(
- validate(message.get(u'upload_ticket', ''), UPLOAD_MESSAGES[1]),
+ expected,
generate_embedded(digests.namespace, digests.items[0]))
def test_pre_upload_invalid_hash(self):
"""Assert that status 400 is returned when the digest is invalid."""
- bad_collection = DigestCollection(
- namespace=endpoint_handlers_api.Namespace())
+ bad_collection = handlers_endpoints.DigestCollection(
+ namespace=handlers_endpoints.Namespace())
bad_digest = hash_content('some stuff', bad_collection.namespace.namespace)
bad_digest = 'g' + bad_digest[1:] # that's not hexadecimal!
bad_collection.items.append(
- endpoint_handlers_api.Digest(digest=bad_digest, size=10))
+ handlers_endpoints.Digest(digest=bad_digest, size=10))
with self.call_should_fail('400'):
self.call_api(
'preupload', self.message_to_dict(bad_collection), 200)
def test_pre_upload_invalid_namespace(self):
"""Assert that status 400 is returned when the namespace is invalid."""
- bad_collection = DigestCollection(
- namespace=endpoint_handlers_api.Namespace(namespace='~tildewhatevs'))
+ bad_collection = handlers_endpoints.DigestCollection(
+ namespace=handlers_endpoints.Namespace(namespace='~tildewhatevs'))
bad_collection.items.append(
generate_digest('pangolin', bad_collection.namespace.namespace))
with self.call_should_fail('400'):
@@ -231,7 +233,8 @@
def test_check_existing_enqueues_tasks(self):
"""Assert that existent entities are enqueued."""
- collection = DigestCollection(namespace=endpoint_handlers_api.Namespace())
+ collection = handlers_endpoints.DigestCollection(
+ namespace=handlers_endpoints.Namespace())
collection.items.append(
generate_digest('some content', collection.namespace))
key = model.entry_key(
@@ -249,7 +252,8 @@
def test_store_inline_ok(self):
"""Assert that inline content storage completes successfully."""
request = self.store_request('sibilance')
- embedded = validate(request.upload_ticket, UPLOAD_MESSAGES[0])
+ embedded = validate(
+ request.upload_ticket, handlers_endpoints.UPLOAD_MESSAGES[0])
key = model.entry_key(embedded['n'], embedded['d'])
# assert that store_inline puts the correct entity into the datastore
@@ -261,12 +265,13 @@
# assert that expected (digest, size) pair is generated by stored content
self.assertEqual(
(embedded['d'].encode('utf-8'), int(embedded['s'])),
- endpoint_handlers_api.hash_content(stored.content, embedded['n']))
+ handlers_endpoints.hash_content(stored.content, embedded['n']))
def test_store_inline_empty_content(self):
"""Assert that inline content storage works when content is empty."""
request = self.store_request('')
- embedded = validate(request.upload_ticket, UPLOAD_MESSAGES[0])
+ embedded = validate(
+ request.upload_ticket, handlers_endpoints.UPLOAD_MESSAGES[0])
key = model.entry_key(embedded['n'], embedded['d'])
# assert that store_inline puts the correct entity into the datastore
@@ -278,7 +283,7 @@
# assert that expected (digest, size) pair is generated by stored content
self.assertEqual(
(embedded['d'].encode('utf-8'), int(embedded['s'])),
- endpoint_handlers_api.hash_content(stored.content, embedded['n']))
+ handlers_endpoints.hash_content(stored.content, embedded['n']))
def test_store_inline_bad_mac(self):
"""Assert that inline content storage fails when token is altered."""
@@ -334,7 +339,8 @@
"""Assert that finalize_gs_upload creates a content entry."""
content = pad_string('empathy')
request = self.store_request(content)
- embedded = validate(request.upload_ticket, UPLOAD_MESSAGES[1])
+ embedded = validate(
+ request.upload_ticket, handlers_endpoints.UPLOAD_MESSAGES[1])
key = model.entry_key(embedded['n'], embedded['d'])
# finalize_gs_upload should put a new ContentEntry into the database
@@ -403,11 +409,12 @@
"""Assert that content retrieval goes off swimmingly in the normal case."""
content = 'Grecian Urn'
request = self.store_request(content)
- embedded = validate(request.upload_ticket, UPLOAD_MESSAGES[0])
+ embedded = validate(
+ request.upload_ticket, handlers_endpoints.UPLOAD_MESSAGES[0])
self.call_api(
'store_inline', self.message_to_dict(request), 200)
- retrieve_request = endpoint_handlers_api.RetrieveRequest(
- digest=embedded['d'], namespace=endpoint_handlers_api.Namespace())
+ retrieve_request = handlers_endpoints.RetrieveRequest(
+ digest=embedded['d'], namespace=handlers_endpoints.Namespace())
response = self.call_api(
'retrieve', self.message_to_dict(retrieve_request), 200)
retrieved = response.json
@@ -417,11 +424,12 @@
"""Assert that content retrieval works for non-memcached DB entities."""
content = 'Isabella, or the Pot of Basil'
request = self.store_request(content)
- embedded = validate(request.upload_ticket, UPLOAD_MESSAGES[0])
+ embedded = validate(
+ request.upload_ticket, handlers_endpoints.UPLOAD_MESSAGES[0])
self.call_api(
'store_inline', self.message_to_dict(request), 200)
- retrieve_request = endpoint_handlers_api.RetrieveRequest(
- digest=embedded['d'], namespace=endpoint_handlers_api.Namespace())
+ retrieve_request = handlers_endpoints.RetrieveRequest(
+ digest=embedded['d'], namespace=handlers_endpoints.Namespace())
memcache.flush_all()
response = self.call_api(
'retrieve', self.message_to_dict(retrieve_request), 200)
@@ -440,14 +448,15 @@
# finalize GS upload
request = preupload_status_to_request(message, content)
- embedded = validate(request.upload_ticket, UPLOAD_MESSAGES[1])
+ embedded = validate(
+ request.upload_ticket, handlers_endpoints.UPLOAD_MESSAGES[1])
self.mock(gcs, 'get_file_info', get_file_info_factory(content))
self.call_api(
'finalize_gs_upload', self.message_to_dict(request), 200)
# retrieve the upload URL
- retrieve_request = endpoint_handlers_api.RetrieveRequest(
- digest=embedded['d'], namespace=endpoint_handlers_api.Namespace())
+ retrieve_request = handlers_endpoints.RetrieveRequest(
+ digest=embedded['d'], namespace=handlers_endpoints.Namespace())
retrieved_response = self.call_api(
'retrieve', self.message_to_dict(retrieve_request), 200)
retrieved = retrieved_response.json
@@ -463,12 +472,13 @@
content = 'Song of the Andoumboulou'
offset = 5
request = self.store_request(content)
- embedded = validate(request.upload_ticket, UPLOAD_MESSAGES[0])
+ embedded = validate(
+ request.upload_ticket, handlers_endpoints.UPLOAD_MESSAGES[0])
self.call_api(
'store_inline', self.message_to_dict(request), 200)
- retrieve_request = endpoint_handlers_api.RetrieveRequest(
+ retrieve_request = handlers_endpoints.RetrieveRequest(
digest=embedded['d'],
- namespace=endpoint_handlers_api.Namespace(),
+ namespace=handlers_endpoints.Namespace(),
offset=offset) # TODO(cmassaro): determine where offsets come from
response = self.call_api(
'retrieve', self.message_to_dict(retrieve_request), 200)
@@ -480,12 +490,13 @@
"""Assert that retrieval fails with status 416 when offset is invalid."""
content = 'Of Man\'s first Disobedience, and the Fruit'
request = self.store_request(content)
- embedded = validate(request.upload_ticket, UPLOAD_MESSAGES[0])
+ embedded = validate(
+ request.upload_ticket, handlers_endpoints.UPLOAD_MESSAGES[0])
self.call_api(
'store_inline', self.message_to_dict(request), 200)
- requests = [endpoint_handlers_api.RetrieveRequest(
+ requests = [handlers_endpoints.RetrieveRequest(
digest=embedded['d'],
- namespace=endpoint_handlers_api.Namespace(),
+ namespace=handlers_endpoints.Namespace(),
offset=offset) for offset in [-1, len(content) + 1]]
for request in requests:
with self.call_should_fail('400'):
@@ -505,11 +516,12 @@
# get the digest
request = preupload_status_to_request(message, content)
- embedded = validate(request.upload_ticket, UPLOAD_MESSAGES[0])
+ embedded = validate(
+ request.upload_ticket, handlers_endpoints.UPLOAD_MESSAGES[0])
# don't upload data; try to retrieve
- retrieve_request = endpoint_handlers_api.RetrieveRequest(
- digest=embedded['d'], namespace=endpoint_handlers_api.Namespace())
+ retrieve_request = handlers_endpoints.RetrieveRequest(
+ digest=embedded['d'], namespace=handlers_endpoints.Namespace())
with self.call_should_fail('404'):
self.call_api(
'retrieve', self.message_to_dict(retrieve_request), 200)
diff --git a/appengine/isolate/handlers_frontend.py b/appengine/isolate/handlers_frontend.py
index 4def64b..3ea3444 100644
--- a/appengine/isolate/handlers_frontend.py
+++ b/appengine/isolate/handlers_frontend.py
@@ -259,7 +259,7 @@
]
-def create_application(debug=False):
+def create_application(debug):
"""Creates the url router.
The basic layouts is as follow:
diff --git a/appengine/isolate/main_backend.py b/appengine/isolate/main_backend.py
index 03528f4..c1e966d 100644
--- a/appengine/isolate/main_backend.py
+++ b/appengine/isolate/main_backend.py
@@ -2,16 +2,18 @@
# Use of this source code is governed by the Apache v2.0 license that can be
# found in the LICENSE file.
-"""This module is the entry point to load the AppEngine instance."""
+"""This modules is imported by AppEngine and defines the 'app' object.
+
+It is a separate file so that application bootstrapping code like ereporter2,
+that shouldn't be done in unit tests, can be done safely. This file must be
+tested via a smoke test.
+"""
import os
import sys
-from google.appengine.ext.appstats import recording
-
-BASE_DIR = os.path.dirname(os.path.abspath(__file__))
-sys.path.insert(0, os.path.join(BASE_DIR, 'third_party'))
-sys.path.insert(0, os.path.join(BASE_DIR, 'components', 'third_party'))
+APP_DIR = os.path.dirname(os.path.abspath(__file__))
+sys.path.insert(0, os.path.join(APP_DIR, 'components', 'third_party'))
from components import ereporter2
from components import utils
@@ -20,18 +22,8 @@
def create_application():
- """Bootstraps the app and creates the url router."""
ereporter2.register_formatter()
- a = handlers_backend.create_application()
- # In theory we'd want to take the output of app_identity.get_application_id().
- # Sadly, this function does an RPC call and may contribute to cause time out
- # on the initial load.
- # Doing it here instead of appengine_config.py reduce the scope of appstats
- # recording. To clarify, this means mapreduces started with mapreduce_jobs.py
- # won't be instrumented, which is actually what we want in practice.
- if utils.is_canary():
- a = recording.appstats_wsgi_middleware(a)
- return a
+ return handlers_backend.create_application(False)
app = create_application()
diff --git a/appengine/isolate/main_frontend.py b/appengine/isolate/main_frontend.py
index f67cad6..72daf57 100644
--- a/appengine/isolate/main_frontend.py
+++ b/appengine/isolate/main_frontend.py
@@ -2,36 +2,36 @@
# Use of this source code is governed by the Apache v2.0 license that can be
# found in the LICENSE file.
-"""This module is the entry point to load the AppEngine instance."""
+"""This modules is imported by AppEngine and defines the 'app' object.
+
+It is a separate file so that application bootstrapping code like ereporter2,
+that shouldn't be done in unit tests, can be done safely. This file must be
+tested via a smoke test.
+"""
import os
import sys
-from google.appengine.ext.appstats import recording
+import endpoints
-BASE_DIR = os.path.dirname(os.path.abspath(__file__))
-sys.path.insert(0, os.path.join(BASE_DIR, 'third_party'))
-sys.path.insert(0, os.path.join(BASE_DIR, 'components', 'third_party'))
+APP_DIR = os.path.dirname(os.path.abspath(__file__))
+sys.path.insert(0, os.path.join(APP_DIR, 'components', 'third_party'))
from components import ereporter2
from components import utils
+import handlers_endpoints
import handlers_frontend
def create_application():
- """Bootstraps the app and creates the url router."""
ereporter2.register_formatter()
- a = handlers_frontend.create_application()
- # In theory we'd want to take the output of app_identity.get_application_id().
- # Sadly, this function does an RPC call and may contribute to cause time out
- # on the initial load.
- # Doing it here instead of appengine_config.py reduce the scope of appstats
- # recording. To clarify, this means mapreduces started with mapreduce_jobs.py
- # won't be instrumented, which is actually what we want in practice.
- if utils.is_canary():
- a = recording.appstats_wsgi_middleware(a)
- return a
+
+ # App that serves HTML pages and old API.
+ frontend = handlers_frontend.create_application(False)
+ # App that serves new endpoints API.
+ api = endpoints.api_server([handlers_endpoints.isolate_api])
+ return frontend, api
-app = create_application()
+frontend_app, endpoints_app = create_application()
diff --git a/appengine/isolate/mapreduce_jobs.py b/appengine/isolate/mapreduce_jobs.py
index 366695a..c8c9b17 100644
--- a/appengine/isolate/mapreduce_jobs.py
+++ b/appengine/isolate/mapreduce_jobs.py
@@ -2,12 +2,16 @@
# Use of this source code is governed by the Apache v2.0 license that can be
# found in the LICENSE file.
-"""Map reduce jobs to update the DB schemas or any other maintenance task."""
+"""Defines the mapreduces, which are used to do one-off mass updates on entities
+and other manually triggered maintenance tasks.
+
+Automatically triggered maintenance tasks should use a task queue on the backend
+instead.
+"""
import logging
from mapreduce import control
-from mapreduce import main
import config
import gcs
@@ -80,7 +84,3 @@
# think that entity exists.
entry.key.delete(use_memcache=True)
logging.error('MR: deleted bad entry\n%s', entry.key.id())
-
-
-# Export mapreduce WSGI application as 'app' for *.yaml routes.
-app = main.APP
diff --git a/appengine/isolate/module-backend.yaml b/appengine/isolate/module-backend.yaml
index c8a1389..5a51776 100644
--- a/appengine/isolate/module-backend.yaml
+++ b/appengine/isolate/module-backend.yaml
@@ -9,7 +9,7 @@
handlers:
- url: /internal/mapreduce(/.*)?
- script: mapreduce_jobs.app
+ script: mapreduce.main.APP
secure: always
login: admin
@@ -19,7 +19,6 @@
login: admin
builtins:
-- appstats: on
- remote_api: on
includes:
