Closed Bug 755265 Opened 12 years ago Closed 12 years ago

Use add_new_release in buildutil

Categories

(Socorro :: Backend, task)

task
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: adrian, Assigned: adrian)

References

Details

Attachments

(2 files)

In socorro/lib/buildutil.py, we insert new data directly into the release_raw table. Now that we have the add_new_release function, we should use it instead.
Blocks: 734095
@Josh: can you please confirm the type of data you expect? 

  product_name > string e.g. 'Firefox'
  version > string e.g. '14.0a1'
  release_channel > string in ['Release', 'Beta', 'Aurora', 'Nightly']
  build_id > int
  platform > string e.g. 'Windows NT', 'Mac OS X'... 
  beta_number > int?? (is this for rapid-betas? )
  repository > string e.g. 'mozilla-central'

When adding a new release from the UI, do we expect the user to know and provide all information listed here?
Target Milestone: 10 → 11
Adrian,

The data formats are correct.  Beta-number is something we've been using since October.  It's "which beta is this ... 1,2,3,4,5?".

Note that betas will NOT have a "b" in the version string.  If that's complicated to do in the mware, I'll have the SP clean them out.  I just realized I need to add code to the SP to normalize upper/lower case anyway.

--Josh Berkus
@Josh: I'm currently having an issue I just don't understand. I'm trying to add a new release, and it keeps telling me the product is not valid. 

Here are the queries I use:

INSERT INTO products
(product_name, sort, rapid_release_version, release_name)
VALUES
(
	'Firefox',
	'0',
	'15.0',
	'firefox'
);

SELECT * FROM add_new_release('Firefox', '15.0a1', 'Nightly', 1234567890, 'Linux', NULL, 'mozilla-central');

And here is the execution in a shell, against an empty database (basically only the role.sql and schema.sql files were run):

integration_test=# INSERT INTO products
integration_test-# (product_name, sort, rapid_release_version, release_name)
integration_test-# VALUES
integration_test-# (
integration_test(# 'Firefox',
integration_test(# '0',
integration_test(# '15.0',
integration_test(# 'firefox'
integration_test(# );
INSERT 0 1
integration_test=# SELECT * FROM add_new_release('Firefox', '15.0a1', 'Nightly', 1234567890, 'Linux', NULL, 'mozilla-central');
ERROR:  Firefox is not a valid product
CONTEXT:  SQL statement "SELECT validate_lookup('products','product_name',product,'product')"
PL/pgSQL function "add_new_release" line 15 at PERFORM

After checking what validate_lookup does, I can't think what the problem is here... The insert went well, I can select from products and see the one I need, etc. Any idea?
Target Milestone: 11 → 12
Target Milestone: 12 → 13
PR: https://github.com/mozilla/socorro/pull/629

About QA, I don't really know how we could test it. It has some impact on the ftpScrapper cron job, as that one inserts new releases. How do we QA that?
Commits pushed to admin-refactor at https://github.com/mozilla/socorro

https://github.com/mozilla/socorro/commit/13120936ffef4b7eba328a8d8633d9c352165d25
Fixes bug 755265 - Made insert_build use the new add_new_release SQL function.

https://github.com/mozilla/socorro/commit/3e2735dd1ce1fd6b90e3903467642c46622bb222
Merge pull request #629 from AdrianGaudebert/755265-use-add-new-release-sql-function

Fixes bug 755265 - Made insert_build use the new add_new_release SQL function.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Commit pushed to admin-refactor at https://github.com/mozilla/socorro

https://github.com/mozilla/socorro/commit/0500832f8815cc240c2583f288a761911a6d0a90
Fixes bug 755265 - Made insert_build use the new add_new_release SQL function.
(In reply to Adrian Gaudebert [:adrian] from comment #4)
> PR: https://github.com/mozilla/socorro/pull/629
> 
> About QA, I don't really know how we could test it. It has some impact on
> the ftpScrapper cron job, as that one inserts new releases. How do we QA
> that?

Adrian, can you describe what the impact is, what behavior has been updated - how does the add_new_release function differ from what we were doing before?
@Matt, there is impact on the ftpScrapper cron job only, the way a new release is added into the database has changed. CC'ing rhelmer for how that can be tested.
Huh?  Why would the FTP-scraper be using add_new_release?  add_new_release is for the admin UI only.
@Josh: the ftpScrapper uses buildutil.insert_build which now uses add_new_release when it discovers a new build. Before this bug, it used to insert directly into releases_raw, and you told me that it should never happen. Please correct me if I'm wrong here!
Adrian,

I was referring to the UI, NOT to the ftpscraper, which is a utility process. 

Importantly, I didn't write that SP with the ftpscraper in mind, so I don't know if it'll work or not.
@Josh: the ftpscrapper used to insert into releases_raw directly, without any sort of verification. Is that wanted? Is it not better to use add_new_release instead? 

Testing the admin UI will come with bug 755230.
Rhelmer, what do you think?

Adrian, please ask rhelmer.
I took a look at the code (both the DB function and the python code change), and I think it should be fine.

There are no errors reported in the logs, one thing that'd be useful to verify is that we've picked up releases that have been pushed since this code went live on stage.
(In reply to Robert Helmer [:rhelmer] from comment #14)
> I took a look at the code (both the DB function and the python code change),
> and I think it should be fine.
> 
> There are no errors reported in the logs, one thing that'd be useful to
> verify is that we've picked up releases that have been pushed since this
> code went live on stage.

From a quick glance at https://crash-stats.allizom.org/products/Firefox/versions/16.0a1/builds I'd say we are.
Thank rhmeler - per comment 15 it's apparent this is working

https://crash-stats.allizom.org/products/Firefox/versions/16.0a1/builds
Status: RESOLVED → VERIFIED
Target Milestone: 13 → 14
Commit pushed to admin-refactor at https://github.com/mozilla/socorro

https://github.com/mozilla/socorro/commit/953e649a93a202510992fe26af3c22f74f3bedc6
Fixes bug 755265 - Made insert_build use the new add_new_release SQL function.
Status: VERIFIED → RESOLVED
Closed: 12 years ago12 years ago
Commit pushed to admin-refactor at https://github.com/mozilla/socorro

https://github.com/mozilla/socorro/commit/0500832f8815cc240c2583f288a761911a6d0a90
Fixes bug 755265 - Made insert_build use the new add_new_release SQL function.
Commit pushed to admin-refactor at https://github.com/mozilla/socorro

https://github.com/mozilla/socorro/commit/811533a18b62d055f4bad2355699367976cb4d47
Fixes bug 755265 - Made insert_build use the new add_new_release SQL function.
Target Milestone: 14 → 16
Commit pushed to master at https://github.com/mozilla/socorro

https://github.com/mozilla/socorro/commit/811533a18b62d055f4bad2355699367976cb4d47
Fixes bug 755265 - Made insert_build use the new add_new_release SQL function.
The nightly build report for FF 16.0a1 show that no builds were found for July 9th & 10th - https://crash-stats.allizom.org/products/Firefox/versions/16.0a1/builds. Is this correct? Looking at the FTP site I see a recent build for Jully 10th.

http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2012-07-10-04-02-28-mozilla-inbound/
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Outstanding pull request on this.  Let's review, land, merge to stage and ship in 16.
Commits pushed to master at https://github.com/mozilla/socorro

https://github.com/mozilla/socorro/commit/62264e131f4830d7f9a3a77b4efe56ca0223aa65
Fixes bug 755265 - Added ignore_duplicates parameter to buildutil.insert_build and called it from ftpscrapper.

https://github.com/mozilla/socorro/commit/9ef5fbe69ed82b93f5d2e188d8467ede343c68aa
Merge pull request #687 from AdrianGaudebert/755265-fix-ftpscrapper

Fixes bug 755265 - Added a try block around buildutil.insert_build to av...
Status: REOPENED → RESOLVED
Closed: 12 years ago12 years ago
Resolution: --- → FIXED
Attached image qa - verified
QA verified - following the steps outlined in comment 21 this looks good to me. Thanks Adrian.

QA verified on stage
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: