999410 - ship it throws ISE 500 when trying to submit a release that exists

Status: RESOLVED FIXED

(Release Engineering :: Release Automation: Other, defect)

Description

[[e-mail] (:bhearsum)]

This happened for Fennec 29.0b10 last night, and I found this traceback in the server logs:
IntegrityError: (IntegrityError) (1062, "Duplicate entry 'Fennec-28.0b10-build1' for key 'PRIMARY'") 'INSERT INTO fennec_release (name, submitter, `submittedAt`, version, `buildNumber`, branch, `mozillaRevision`, `l10nChangesets`, `dashboardCheck`, ready, complete, status, `mozillaRelbranch`) VALUES (%s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s)' ('Fennec-28.0b10-build1', '[e-mail]', datetime.datetime(2014, 4, 22, 4, 16, 47, 97674), '28.0b10', 1, 'releases/mozilla-beta', 'aa70a6ce178a', '{\r\n  "ar": {\r\n    "revision": "9c4b301989ca",\r\n    "platforms": ["android"]\r\n  },\r\n  "be": {\r\n    "revision": "7cddb10a8ece",\r\n    "platforms": ["android"]\r\n  },\r\n  "ca": {\r\n    "revision": "52097839a3fc",\r\n    "platforms": ["android", "android-multilocale"]\r\n  },\r\n  "cs": {\r\n    "revision": "9262aab01a97",\r\n    "platforms": ["android", "android-multilocale"]\r\n  },\r\n  "cy": {\r\n    "revision": "53f662270dc2",\r\n    "platforms": ["android"]\r\n  },\r\n  "da": {\r\n    "revision": "186c760dbdc5",\r\n    "platforms": ["android", "android-multilocale"]\r\n  },\r\n  "de": {\r\n    "revision": "543ddd9420b6",\r\n    "platforms": ["android", "android-multilocale"]\r\n  },\r\n  "en-GB": {\r\n    "revision": "acaf09dc9a9b",\r\n    "platforms": ["android", "android-multilocale"]\r\n  },\r\n  "es-AR": {\r\n    "revision": "03c68802bdc4",\r\n    "platforms": ["android"]\r\n  },\r\n  "es-ES": {\r\n    "revision": "2b52ea881354",\r\n    "platforms": ["android", "android-multilocale"]\r\n  },\r\n  "et": {\r\n    "revision": "24cdf4da7600",\r\n    "platforms": ["android", "android-multilocale"]\r\n  },\r\n  "fi": {\r\n    "revision": "bfc99fac1ed8",\r\n    "platforms": ["android", "android-multilocale"]\r\n  },\r\n  "fr": {\r\n    "revision": "1e920f399c55",\r\n    "platforms": ["android", "android-multilocale"]\r\n  },\r\n  "fy-NL": {\r\n    "revision": "0bb46089e086",\r\n    "platforms": ["android"]\r\n  },\r\n  "ga-IE": {\r\n    "revision": "e66f00d3c8f8",\r\n    "platforms": ["android", "android-multilocale"]\r\n  },\r\n  "gl": {\r\n    "revision": "6a40cee822a6",\r\n    "platforms": ["android"]\r\n  },\r\n  "hu": {\r\n    "revision": "1c61db941066",\r\n    "platforms": ["android", "android-multilocale"]\r\n  },\r\n  "it": {\r\n    "revision": "d5608e09bd7f",\r\n    "platforms": ["android", "android-multilocale"]\r\n  },\r\n  "ja": {\r\n    "revision": "92c12b4d7e3b",\r\n    "platforms": ["android", "android-multilocale"]\r\n  },\r\n  "ko": {\r\n    "revision": "5205f455eeff",\r\n    "platforms": ["android", "android-multilocale"]\r\n  },\r\n  "lt": {\r\n    "revision": "9e269c73722f",\r\n    "platforms": ["android", "android-multilocale"]\r\n  },\r\n  "nb-NO": {\r\n    "revision": "b1d7ecff7e73",\r\n    "platforms": ["android", "android-multilocale"]\r\n  },\r\n  "nl": {\r\n    "revision": "3ba03f089b07",\r\n    "platforms": ["android", "android-multilocale"]\r\n  },\r\n  "pa-IN": {\r\n    "revision": "4afbb88e0ccf",\r\n    "platforms": ["android"]\r\n  },\r\n  "pl": {\r\n    "revision": "c23ab28ff636",\r\n    "platforms": ["android", "android-multilocale"]\r\n  },\r\n  "pt-BR": {\r\n    "revision": "5aec354bb7a3",\r\n    "platforms": ["android", "android-multilocale"]\r\n  },\r\n  "pt-PT": {\r\n    "revision": "04da0167acae",\r\n    "platforms": ["android", "android-multilocale"]\r\n  },\r\n  "ro": {\r\n    "revision": "cdbf50ab417c",\r\n    "platforms": ["android", "android-multilocale"]\r\n  },\r\n  "ru": {\r\n    "revision": "b6dd363329ff",\r\n    "platforms": ["android", "android-multilocale"]\r\n  },\r\n  "sk": {\r\n    "revision": "6ca5dce39616",\r\n    "platforms": ["android", "android-multilocale"]\r\n  },\r\n  "sl": {\r\n    "revision": "ff1548822be0",\r\n    "platforms": ["android", "android-multilocale"]\r\n  },\r\n  "sv-SE": {\r\n    "revision": "73044845fab8",\r\n    "platforms": ["android", "android-multilocale"]\r\n  },\r\n  "th": {\r\n    "revision": "b5a7ec14fabf",\r\n    "platforms": ["android", "android-multilocale"]\r\n  },\r\n  "tr": {\r\n    "revision": "02e80792f075",\r\n    "platforms": ["android", "android-multilocale"]\r\n  },\r\n  "uk": {\r\n    "revision": "aa0a9532e3bb",\r\n    "platforms": ["android", "android-multilocale"]\r\n  },\r\n  "zh-CN": {\r\n    "revision": "30298cb827a2",\r\n    "platforms": ["android", "android-multilocale"]\r\n  },\r\n  "zh-TW": {\r\n    "revision": "a3bd067aab48",\r\n    "platforms": ["android", "android-multilocale"]\r\n  }\r\n}', 1, 0, 0, '', None)

We should probably return a 400 instead.

Comment 1

[[e-mail] (:bhearsum)]

Relevant part of the traceback:
Traceback (most recent call last):
  File "/data/www/ship-it.mozilla.org/release-kickoff/vendor/lib/python/flask/app.py", line 1687, in wsgi_app
    response = self.full_dispatch_request()
  File "/data/www/ship-it.mozilla.org/release-kickoff/vendor/lib/python/flask/app.py", line 1360, in full_dispatch_request
    rv = self.handle_user_exception(e)
  File "/data/www/ship-it.mozilla.org/release-kickoff/vendor/lib/python/flask/app.py", line 1358, in full_dispatch_request
    rv = self.dispatch_request()
  File "/data/www/ship-it.mozilla.org/release-kickoff/vendor/lib/python/flask/app.py", line 1344, in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
  File "/data/www/ship-it.mozilla.org/release-kickoff/vendor/lib/python/flask/views.py", line 83, in view
    return self.dispatch_request(*args, **kwargs)
  File "/data/www/ship-it.mozilla.org/release-kickoff/vendor/lib/python/flask/views.py", line 150, in dispatch_request
    return meth(*args, **kwargs)
  File "/data/www/ship-it.mozilla.org/release-kickoff/kickoff/views/submit.py", line 57, in post
    db.session.commit()
  File "/usr/lib64/python2.6/site-packages/sqlalchemy/orm/scoping.py", line 113, in do
    return getattr(self.registry(), name)(*args, **kwargs)

Comment 2

[[e-mail] (:bhearsum)]

Attachment: 500-404-release-kickoff.diff

Check for existence before attempting to create the release.

Comment 3

[Rail Aliiev [:rail]]

    rail │ bhearsum: I'm trying to be stricter! I think their is a very little chance for a race condition in your ship-it patch ;)
bhearsum │ rail: yeah
bhearsum │ i thought about that
bhearsum │ i could catch IntegrityError during commit()
    rail │ meh, since the chance is really low
bhearsum │ but i'm not sure how to distinguish a "already exists" IntegrityError from some other violation

Comment 4

[[e-mail] (:bhearsum)]

Comment on attachment 8410358 [details] [diff] [review]
500-404-release-kickoff.diff

Checked in. Going to test in dev a little bit before pushing to stage/prod.

Comment 5

[[e-mail] (:bhearsum)]

(In reply to Ben Hearsum [:bhearsum] from comment #4)
> Comment on attachment 8410358 [details] [diff] [review]
> 500-404-release-kickoff.diff
> 
> Checked in. Going to test in dev a little bit before pushing to stage/prod.

Actually, can't test on dev or stage becausue of bug 992940. I pushed to prod, and it looks fine there...