launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/no-job-oops-prefix into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/no-job-oops-prefix/+merge/104315

Now that we use hash-based OOPS IDs, each process no longer requires its own oops_prefix -- they're only important to identify multiple instances of the same service within a single environment (appservers, librarian, codehosting, codebrowse).

The unnecessary prefixes were eliminated from prod configs last year, and the last few are being moved away in <https://code.launchpad.net/~wgrant/lp-production-configs/no-section-oops-prefix/+merge/104314>. This branch removes the non-global oops_prefix config keys. There should be no difference in production, but the test configs still specify service-specific prefixes so there is some test fallout.
-- 
https://code.launchpad.net/~wgrant/launchpad/no-job-oops-prefix/+merge/104315
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/no-job-oops-prefix into lp:launchpad.

=== modified file 'configs/development/launchpad-lazr.conf'
--- configs/development/launchpad-lazr.conf	2012-03-20 20:50:47 +0000
+++ configs/development/launchpad-lazr.conf	2012-05-02 01:50:24 +0000
@@ -9,7 +9,6 @@
 run_parts_location: none
 
 [branchscanner]
-oops_prefix: BS
 error_dir: /var/tmp/codehosting.test
 
 [builddmaster]
@@ -19,13 +18,11 @@
 
 [bzr_lpserve]
 error_dir: /var/tmp/codehosting.test
-oops_prefix: BZR
 
 [canonical]
 show_tracebacks: True
 
 [checkwatches]
-oops_prefix: XCW
 error_dir: /var/tmp/lperr
 sync_debbugs_comments: True
 
@@ -41,7 +38,6 @@
 launchpad_root: https://code.launchpad.dev/
 secret_path: configs/development/codebrowse-secret
 error_dir: /var/tmp/codebrowse.launchpad.dev/errors
-oops_prefix: CB
 
 [codehosting]
 launch: True
@@ -56,7 +52,6 @@
 host_key_pair_path: lib/lp/codehosting/sshserver/tests/keys
 port: tcp:5022:interface=127.0.0.88
 error_dir: /var/tmp/codehosting.test
-oops_prefix: SMPSSH
 bzr_lp_prefix: lp://dev/
 lp_url_hosts: dev
 access_log: /var/tmp/bazaar.launchpad.dev/codehosting-access.log
@@ -73,7 +68,6 @@
 
 [codeimportworker]
 error_dir: /var/tmp/codehosting.test
-oops_prefix: CIW
 
 [commercial]
 voucher_proxy_url: http://launchpad.dev
@@ -100,7 +94,6 @@
 cdimage_file_list_url: file:lib/lp/registry/tests/ubuntu-releases.testdata
 
 [distroseriesdifferencejob]
-oops_prefix: DSDJ
 error_dir: /var/tmp/soyuz.test
 
 [error_reports]
@@ -121,11 +114,9 @@
 public_host: keyserver.launchpad.dev
 
 [initializedistroseries]
-oops_prefix: IDSJ
 error_dir: /var/tmp/soyuz.test
 
 [IPlainPackageCopyJobSource]
-oops_prefix: PCJ
 error_dir: /var/tmp/soyuz.test
 
 [launchpad]
@@ -159,7 +150,6 @@
 restricted_download_port: 58085
 restricted_download_url: http://launchpad.dev:58085/
 use_https = False
-oops_prefix: L
 error_dir: /var/tmp/codehosting.test
 
 [librarian_server]
@@ -208,10 +198,8 @@
 
 [merge_proposal_jobs]
 error_dir: /var/tmp/codehosting.test
-oops_prefix: DMPJ
 
 [packaging_translations]
-oops_prefix: DPT
 error_dir: /var/tmp/lperr
 
 [personalpackagearchive]
@@ -240,7 +228,6 @@
 
 [reclaimbranchspace]
 error_dir: /var/tmp/codehosting.test
-oops_prefix: RBS
 
 [rosetta]
 global_suggestions_enabled: True
@@ -248,11 +235,9 @@
 
 [rosettabranches]
 error_dir: /var/tmp/rosettabranches.test
-oops_prefix: RSBR
 
 [poimport]
 error_dir: /var/tmp/poimport
-oops_prefix: POI
 
 [process_apport_blobs]
 error_dir: /var/tmp/lperr
@@ -262,26 +247,20 @@
 
 [supermirror_puller]
 error_dir: /var/tmp/codehosting.test
-oops_prefix: SMP
 
 [supermirror_import_puller]
 error_dir: /var/tmp/codehosting.test
-oops_prefix: ISMP
 
 [supermirror_mirror_puller]
 error_dir: /var/tmp/codehosting.test
-oops_prefix: MSMP
 
 [supermirror_upload_puller]
 error_dir: /var/tmp/codehosting.test
-oops_prefix: USMP
 
 [translations_export_to_branch]
 error_dir: /var/tmp/translations_export_to_branch
-oops_prefix: TEB
 
 [upgrade_branches]
-oops_prefix: UBJD
 error_dir: /var/tmp/codehosting.test
 
 [uploader]

=== modified file 'configs/testrunner/launchpad-lazr.conf'
--- configs/testrunner/launchpad-lazr.conf	2012-04-27 16:22:05 +0000
+++ configs/testrunner/launchpad-lazr.conf	2012-05-02 01:50:24 +0000
@@ -9,7 +9,6 @@
 cron_control_url: file:lib/lp/services/scripts/tests/cronscripts.ini
 
 [branchscanner]
-oops_prefix: TSMS
 error_dir: /var/tmp/lperr.test
 
 [builddmaster]
@@ -18,7 +17,6 @@
 
 [checkwatches]
 sync_debbugs_comments: True
-oops_prefix: TCW
 error_dir: /var/tmp/lperr.test
 
 [codehosting]
@@ -26,7 +24,6 @@
 host_key_pair_path: lib/lp/codehosting/sshserver/tests/keys
 port: tcp:22222:interface=bazaar.launchpad.dev
 error_dir: /var/tmp/codehosting.test
-oops_prefix: SMPSSH
 access_log: /tmp/test-codehosting-access.log
 internal_branch_by_id_root: file:///var/tmp/bazaar.launchpad.dev/mirrors
 
@@ -160,15 +157,12 @@
 port: 11242
 
 [merge_proposal_jobs]
-oops_prefix: TMPJ
 error_dir: /var/tmp/codehosting.test
 
 [packaging_translations]
-oops_prefix: TPT
 error_dir: /var/tmp/lperr.test
 
 [upgrade_branches]
-oops_prefix: TUB
 error_dir: /var/tmp/codehosting.test
 
 [personalpackagearchive]
@@ -179,11 +173,9 @@
 
 [poimport]
 error_dir: /var/tmp/poimport.test
-oops_prefix: TPOI
 
 [process_apport_blobs]
 dbuser: process-apport-blobs
-oops_prefix: TAPPORTBLOB
 error_dir: /var/tmp/lperr.test
 
 [rabbitmq]
@@ -199,31 +191,25 @@
 uri: none
 
 [request_daily_builds]
-oops_prefix: TRDB
 error_dir: /var/tmp/lperr.test
 
 [rosetta]
 generate_templates: True
 
 [rosettabranches]
-oops_prefix: TRSBR
 error_dir: /var/tmp/rosettabranches.test
 
 [sendbranchmail]
-oops_prefix: BM
 error_dir: /var/tmp/codehosting.test
 
 [supermirror_import_puller]
 error_dir: /var/tmp/codehosting.test
-oops_prefix: TISMP
 
 [supermirror_mirror_puller]
 error_dir: /var/tmp/codehosting.test
-oops_prefix: TMSMP
 
 [supermirror_upload_puller]
 error_dir: /var/tmp/codehosting.test
-oops_prefix: TUSMP
 
 [uploader]
 default_recipient_name: Root

=== modified file 'lib/lp/services/config/schema-lazr.conf'
--- lib/lp/services/config/schema-lazr.conf	2012-04-27 16:22:05 +0000
+++ lib/lp/services/config/schema-lazr.conf	2012-05-02 01:50:24 +0000
@@ -46,9 +46,6 @@
 # See [error_reports].
 error_dir: none
 
-# See [error_reports].
-oops_prefix: none
-
 
 [builddmaster]
 # The database user which will be used by this process.
@@ -107,8 +104,6 @@
 # Configuration for spawned Bazaar subprocesses.
 [bzr_lpserve]
 # See [error_reports].
-oops_prefix: none
-# See [error_reports].
 error_dir: none
 
 
@@ -140,9 +135,6 @@
 # See [error_reports].
 error_dir: none
 
-# See [error_reports].
-oops_prefix: none
-
 # datatype: integer
 batch_query_threshold: 10
 
@@ -215,9 +207,6 @@
 # See [error_reports].
 error_dir: none
 
-# See [error_reports].
-oops_prefix: none
-
 
 [codehosting]
 # datatype: string
@@ -255,9 +244,6 @@
 # datatype: string
 codehosting_endpoint: none
 
-# See [error_reports].
-oops_prefix: none
-
 # datatype: string
 bzr_lp_prefix: lp:
 
@@ -433,9 +419,6 @@
 # See [error_reports].
 error_dir: /var/tmp/codehosting.test/
 
-# See [error_reports].
-oops_prefix: none
-
 
 [codeimportworker]
 # This code is used by the code-import-worker-monitor which lives in
@@ -460,9 +443,6 @@
 # See [error_reports].
 error_dir: /var/tmp/codehosting.test/
 
-# See [error_reports].
-oops_prefix: none
-
 
 [commercial]
 # URL for salesforce proxy.
@@ -491,9 +471,6 @@
 # See [error_reports].
 error_dir: none
 
-# See [error_reports].
-oops_prefix: none
-
 
 [cveupdater]
 # The database user which will be used by this process.
@@ -615,9 +592,6 @@
 # See [error_reports].
 error_dir: none
 
-# See [error_reports].
-oops_prefix: none
-
 
 [error_reports]
 # A prefix for "OOPS" codes for this process instance.
@@ -849,9 +823,6 @@
 # See [error_reports].
 error_dir: none
 
-# See [error_reports].
-oops_prefix: none
-
 
 [karmacacheupdater]
 # The database user which will be used by this process.
@@ -1195,8 +1166,6 @@
 use_https = True
 
 # See [error_reports].
-# Must be unique per librarian instance.
-oops_prefix: none
 # Must be set per librarian instance.
 error_dir: none
 
@@ -1318,9 +1287,6 @@
 # See [error_reports].
 error_dir: none
 
-# See [error_reports].
-oops_prefix: none
-
 # Whether Mailman should be built if it is not already.
 # datatype: boolean
 build: false
@@ -1447,9 +1413,6 @@
 # See [error_reports].
 error_dir: none
 
-# See [error_reports].
-oops_prefix: none
-
 
 [person_notification]
 # User for person notification db access
@@ -1498,8 +1461,6 @@
 statement_timeout: 300
 
 # See [error_reports].
-oops_prefix: none
-# See [error_reports].
 error_dir: none
 
 
@@ -1510,9 +1471,6 @@
 # datatype: string
 authentication_endpoint: none
 
-# See [error_reports].
-oops_prefix: none
-
 # see [error_reports].
 error_dir: none
 
@@ -1555,7 +1513,6 @@
 # The database user which will be used by this process.
 # datatype: string
 dbuser: process-apport-blobs
-oops_prefix: none
 error_dir: none
 
 
@@ -1618,13 +1575,10 @@
 
 # See [error_reports].
 error_dir: none
-# See [error_reports].
-oops_prefix: none
 
 [request_daily_builds]
 dbuser: request-daily-builds
 error_dir: none
-oops_prefix: none
 
 
 [revisionkarma]
@@ -1675,23 +1629,16 @@
 # See [error_reports].
 error_dir: none
 
-# See [error_reports].
-oops_prefix: none
-
 
 [translations_export_to_branch]
 # See [error_reports].
 error_dir: none
 
-# See [error_reports].
-oops_prefix: none
-
 
 [sendbranchmail]
 # The database user which will be used by this process.
 # datatype: string
 dbuser: send-branch-mail
-oops_prefix: none
 error_dir: none
 
 [pofile_stats]
@@ -1701,9 +1648,6 @@
 # See [error_reports].
 error_dir: none
 
-# See [error_reports].
-oops_prefix: none
-
 
 # For the personal standing updater cron script.
 [standingupdater]
@@ -1736,33 +1680,21 @@
 # See [error_reports].
 error_dir: none
 
-# See [error_reports].
-oops_prefix: none
-
 
 [supermirror_import_puller]
 # See [error_reports].
 error_dir: none
 
-# See [error_reports].
-oops_prefix: none
-
 
 [supermirror_mirror_puller]
 # See [error_reports].
 error_dir: none
 
-# See [error_reports].
-oops_prefix: none
-
 
 [supermirror_upload_puller]
 # See [error_reports].
 error_dir: none
 
-# See [error_reports].
-oops_prefix: none
-
 
 [targetnamecacheupdater]
 # The database user which will be used by this process.
@@ -1794,9 +1726,6 @@
 # See [error_reports].
 error_dir: none
 
-# See [error_reports].
-oops_prefix: none
-
 
 [uploader]
 # The database user which will be used by this process.
@@ -1935,9 +1864,6 @@
 # See [error_reports].
 error_dir: none
 
-# See [error_reports].
-oops_prefix: none
-
 [IQuestionEmailJobSource]
 # This section is used by cronscripts/process-job-source.py.
 module: lp.answers.interfaces.questionjob

=== modified file 'lib/lp/services/job/runner.py'
--- lib/lp/services/job/runner.py	2012-04-26 19:42:04 +0000
+++ lib/lp/services/job/runner.py	2012-05-02 01:50:24 +0000
@@ -607,9 +607,8 @@
 
     def main(self):
         section = self.config_section
-        if (getattr(section, 'error_dir', None) is not None
-            and getattr(section, 'oops_prefix', None) is not None):
-            # If the two variables are not set, we will let the error
+        if getattr(section, 'error_dir', None) is not None:
+            # If the error_dir is not set, we will let the error
             # utility default to using the [error_reports] config.
             errorlog.globalErrorUtility.configure(self.config_name)
         job_source = getUtility(self.source_interface)

=== modified file 'lib/lp/services/job/tests/test_runner.py'
--- lib/lp/services/job/tests/test_runner.py	2012-04-20 19:42:50 +0000
+++ lib/lp/services/job/tests/test_runner.py	2012-05-02 01:50:24 +0000
@@ -662,10 +662,9 @@
             def runFromSource(cls, source, dbuser, logger):
                 expected_config = errorlog.ErrorReportingUtility()
                 expected_config.configure('merge_proposal_jobs')
-                # Check that the unique oops token was applied.
                 self.assertEqual(
-                    errorlog.globalErrorUtility.oops_prefix,
-                    expected_config.oops_prefix)
+                    'T-merge_proposal_jobs',
+                    errorlog.globalErrorUtility.oops_prefix)
                 return cls()
 
             completed_jobs = []

=== modified file 'lib/lp/services/webapp/errorlog.py'
--- lib/lp/services/webapp/errorlog.py	2012-03-06 23:39:08 +0000
+++ lib/lp/services/webapp/errorlog.py	2012-05-02 01:50:24 +0000
@@ -323,12 +323,9 @@
         # Constants:
         self._oops_config.template['branch_nick'] = versioninfo.branch_nick
         self._oops_config.template['revno'] = versioninfo.revno
-        reporter = config[section_name].oops_prefix
-        if reporter is None:
-            # Unconfigured section - make up a reporter slightly more useful
-            # than e.g 'T' or 'LPNET'
-            reporter = (config[self._default_config_section].oops_prefix +
-                '-' + section_name)
+        reporter = config[self._default_config_section].oops_prefix
+        if section_name != self._default_config_section:
+            reporter = '%s-%s' % (reporter, section_name)
         self._oops_config.template['reporter'] = reporter
         # Should go in an HTTP module.
         self._oops_config.template['req_vars'] = {}

=== modified file 'lib/lp/services/webapp/tests/test_errorlog.py'
--- lib/lp/services/webapp/tests/test_errorlog.py	2012-01-01 02:58:52 +0000
+++ lib/lp/services/webapp/tests/test_errorlog.py	2012-05-02 01:50:24 +0000
@@ -161,8 +161,7 @@
         # Some external processes may use another config section to
         # provide the error log configuration.
         utility.configure(section_name='branchscanner')
-        self.assertEqual(config.branchscanner.oops_prefix,
-            utility.oops_prefix)
+        self.assertEqual('T-branchscanner', utility.oops_prefix)
         self.assertEqual(config.branchscanner.error_dir,
             utility._oops_datedir_repo.root)