mozilla-releng/balrog

Add support for a disableBackgroundUpdates XML parameter

Closed this issue · 10 comments

Much like the existing disableBITS parameter that can be sent in the XML to prevent BITS from being used to download an update, we would like a disableBackground parameter that should prevent the update from being downloaded in the background (i.e. initiated when Firefox is not running).

Bugzilla link for the Firefox half of the work

We decided to change the name of this to disableBackgroundUpdates, to minimize confusion.

@sarah-clements is going to take a look at this. Implementation should be straightforward. You'll need to:

You should be able to get a local dev environment with docker-compose up, and run tests with a simple tox command.

It's also generally a good idea to test this by hand, which will be slightly more involved. You'll want to:

@bhearsum Should the disableBackgroundUpdates entry be boolean with enum: true, same as disableBTS?

Also I'm hitting a couple of issues while trying to manually test this:

  1. I can't seem to get localhost:9000 port to work - more notes on that in our relounge channel.
  2. When I go to the balrog admin UI and search for a release, I don't see anything greater than v77 and the schema looks different... I don't see updateLine. Are older releases using an older version of the schema?

@bhearsum Should the disableBackgroundUpdates entry be boolean with enum: true, same as disableBTS?

Yup. There's no use case for setting it false, so it's true or absent.

Also I'm hitting a couple of issues while trying to manually test this:

1. I can't seem to get localhost:9000 port to work - more notes on that in our relounge channel.

I think we sorted this out elsewhere.

2. When I go to the balrog admin UI and search for a release, I don't see anything greater than v77 and the schema looks different... I don't see `updateLine`. Are older releases using an older version of the schema?

This is....expected at the moment (#1843 will fix it, but it's not in production yet).

In the meantime, you can manually add https://aus-api.mozilla.org/api/v1/releases/Firefox-88.0-build1 to your instance. On https://localhost:9000/releases/create/v2 paste in the raw json from the previous link, use Firefox-88.0-build1 for the name and Firefox for `product. Then click the save icon.

@bhearsum I'm hitting a confusing error when trying to test this pr, after manually adding firefox 88 to balrog. If I try to add disableBackgroundTests: true like so:

Screen Shot 2021-04-19 at 4 58 12 PM

I see this error in stdout:

Traceback (most recent call last):
balrogadmin_1  |   File "/usr/local/lib/python3.8/site-packages/flask/app.py", line 2447, in wsgi_app
balrogadmin_1  |     response = self.full_dispatch_request()
balrogadmin_1  |   File "/usr/local/lib/python3.8/site-packages/flask/app.py", line 1952, in full_dispatch_request
balrogadmin_1  |     rv = self.handle_user_exception(e)
balrogadmin_1  |   File "/usr/local/lib/python3.8/site-packages/flask/app.py", line 1822, in handle_user_exception
balrogadmin_1  |     return handler(e)
balrogadmin_1  |   File "/app/src/auslib/web/admin/base.py", line 102, in blob_validation_error
balrogadmin_1  |     return problem(400, "Bad Request", "Invalid Blob", ext={"exception": error.errors})
balrogadmin_1  |   File "/app/src/auslib/web/admin/views/problem.py", line 47, in problem
balrogadmin_1  |     return Response(response=json.dumps(problem_response), status=status, mimetype=mimetype, content_type=content_type, headers=headers)
balrogadmin_1  |   File "/usr/local/lib/python3.8/json/__init__.py", line 231, in dumps
balrogadmin_1  |     return _default_encoder.encode(obj)
balrogadmin_1  |   File "/usr/local/lib/python3.8/json/encoder.py", line 199, in encode
balrogadmin_1  |     chunks = self.iterencode(o, _one_shot=True)
balrogadmin_1  |   File "/usr/local/lib/python3.8/json/encoder.py", line 257, in iterencode
balrogadmin_1  |     return _iterencode(o, 0)
balrogadmin_1  |   File "/usr/local/lib/python3.8/json/encoder.py", line 179, in default
balrogadmin_1  |     raise TypeError(f'Object of type {o.__class__.__name__} '
balrogadmin_1  | TypeError: Object of type set is not JSON serializable

Am missing something obvious?

Hm, that looks like a red herring sort of error - as far as I can tell that's an exception that happened while handling another one. The real problem is hinted at in the traceback:

balrogadmin_1 | return problem(400, "Bad Request", "Invalid Blob", ext={"exception": error.errors})

We can probably get the real exception by adding a log.warning(error.errors) above https://github.com/mozilla-releng/balrog/blob/main/src/auslib/web/admin/base.py#L102 (we should also fix that exception handling code, but it doesn't look trivial).

Based on your patch, I suspect this may actually fix itself though. If you modified the yml file without also modifying a python file, it probably didn't pick up the change to the yml. If you try again today it may just work.

I hit this error again, but completely missed another exception that was first caught. Seems that there is some sort of rule in apprelease.validate that you can only have two entries per field, so that's why I couldn't add disableBackgroundUpdates: true as I did in the previous screen shot. I added a new fields object, updated it and refresh and can see that it worked:

Screen Shot 2021-04-20 at 4 49 22 PM

Huh, that is very surprising to me...I'd be curious exactly what that exception is; but I'm glad it's working now!

@bhearsum this was the full traceback. I tried digging into that bit of code eod yesterday, but I don't have enough context of what the validate function in that file is supposed to be doing (what/why about the rules). Can dig into this more with you on Monday, if needed.

Traceback (most recent call last):
balrogadmin_1  |   File "/usr/local/lib/python3.8/site-packages/flask/app.py", line 1950, in full_dispatch_request
balrogadmin_1  |     rv = self.dispatch_request()
balrogadmin_1  |   File "/usr/local/lib/python3.8/site-packages/flask/app.py", line 1936, in dispatch_request
balrogadmin_1  |     return self.view_functions[rule.endpoint](**req.view_args)
balrogadmin_1  |   File "/usr/local/lib/python3.8/site-packages/connexion/decorators/decorator.py", line 48, in wrapper
balrogadmin_1  |     response = function(request)
balrogadmin_1  |   File "/usr/local/lib/python3.8/site-packages/connexion/decorators/uri_parsing.py", line 144, in wrapper
balrogadmin_1  |     response = function(request)
balrogadmin_1  |   File "/usr/local/lib/python3.8/site-packages/connexion/decorators/validation.py", line 184, in wrapper
balrogadmin_1  |     response = function(request)
balrogadmin_1  |   File "/usr/local/lib/python3.8/site-packages/connexion/decorators/validation.py", line 384, in wrapper
balrogadmin_1  |     return function(request)
balrogadmin_1  |   File "/usr/local/lib/python3.8/site-packages/connexion/decorators/response.py", line 103, in wrapper
balrogadmin_1  |     response = function(request)
balrogadmin_1  |   File "/usr/local/lib/python3.8/site-packages/connexion/decorators/parameter.py", line 121, in wrapper
balrogadmin_1  |     return function(**kwargs)
balrogadmin_1  |   File "/app/src/auslib/web/admin/views/releases_v2.py", line 44, in set_release
balrogadmin_1  |     new_data_versions = releases.set_release(
balrogadmin_1  |   File "/app/src/auslib/services/releases.py", line 566, in set_release
balrogadmin_1  |     createBlob(blob).validate(product or current_product, app.config["WHITELISTED_DOMAINS"])
balrogadmin_1  |   File "/app/src/auslib/blobs/apprelease.py", line 1160, in validate
balrogadmin_1  |     raise BlobValidationError("Multiple values found for updateLine items: {}".format(conflicting_values), conflicts)
balrogadmin_1  | auslib.errors.BlobValidationError: Multiple values found for updateLine items: detailsURL, type
balrogadmin_1  | 
balrogadmin_1  | During handling of the above exception, another exception occurred:
balrogadmin_1  | 
balrogadmin_1  | Traceback (most recent call last):
balrogadmin_1  |   File "/usr/local/lib/python3.8/site-packages/flask/app.py", line 2447, in wsgi_app
balrogadmin_1  |     response = self.full_dispatch_request()
balrogadmin_1  |   File "/usr/local/lib/python3.8/site-packages/flask/app.py", line 1952, in full_dispatch_request
balrogadmin_1  |     rv = self.handle_user_exception(e)
balrogadmin_1  |   File "/usr/local/lib/python3.8/site-packages/flask/app.py", line 1822, in handle_user_exception
balrogadmin_1  |     return handler(e)
balrogadmin_1  |   File "/app/src/auslib/web/admin/base.py", line 102, in blob_validation_error
balrogadmin_1  |     return problem(400, "Bad Request", "Invalid Blob", ext={"exception": error.errors})
balrogadmin_1  |   File "/app/src/auslib/web/admin/views/problem.py", line 47, in problem
balrogadmin_1  |     return Response(response=json.dumps(problem_response), status=status, mimetype=mimetype, content_type=content_type, headers=headers)
balrogadmin_1  |   File "/usr/local/lib/python3.8/json/__init__.py", line 231, in dumps
balrogadmin_1  |     return _default_encoder.encode(obj)
balrogadmin_1  |   File "/usr/local/lib/python3.8/json/encoder.py", line 199, in encode
balrogadmin_1  |     chunks = self.iterencode(o, _one_shot=True)
balrogadmin_1  |   File "/usr/local/lib/python3.8/json/encoder.py", line 257, in iterencode
balrogadmin_1  |     return _iterencode(o, 0)
balrogadmin_1  |   File "/usr/local/lib/python3.8/json/encoder.py", line 179, in default
balrogadmin_1  |     raise TypeError(f'Object of type {o.__class__.__name__} '
balrogadmin_1  | TypeError: Object of type set is not JSON serializable

I noticed something else, while trying this again. This time this error included disableBackgroundUpdates, because I had previously added it as another field entry per the screenshot above: balrogadmin_1 | auslib.errors.BlobValidationError: Multiple values found for updateLine items: disableBackgroundUpdates, detailsURL, type whereas the first time it was only detailsUrl and type. Also, if I try just modifying a value in an existing field, such as changing "type": "minor" to "major", I hit the same error. Maybe there's a bug when trying to update existing fields?

Ah, okay! So it sounds like you had multiple updateLines entries with the same fields - which means we don't know which one to pick. I'm guessing you copy/pasted a block or something. This certainly makes me certain this patch is good; let's merge it!

Ah, okay! So it sounds like you had multiple updateLines entries with the same fields - which means we don't know which one to pick. I'm guessing you copy/pasted a block or something. This certainly makes me certain this patch is good; let's merge it!

Well no, all I did initially was manually edit an existing fields entry per the screen shot in this comment. I only added a new fields entry specifically for disableBackgroundUpdates: true after that didn't work.