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).
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:
- Add a new entry to the AppRelease schema for this parameter:
- Add a couple of new tests for the blob rendering, similar to what we have for
disableBITS
:balrog/tests/blobs/test_apprelease.py
Line 3536 in 8668813
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:
- Use an URL like http://localhost:9010/update/6/Firefox/83.0/20200927215113/WINNT_x86_64-msvc-x64/en-US/release/Windows_NT%2010.0.0.0.18363.1110%20(x64)/ISET:SSE4_1,MEM:8191/default/default/update.xml?force=1 to make sure your local Balrog instance is running -- note the Firefox version being served, and whether or not
disableBackgroundUpdates
is present in the XML. - Find the Release on https://localhost:9000/releases, eg:
Firefox-88.0-build1
- Click
Update
- In the first item of the
updateLine
key, add a newfield
entry with a key ofdisableBackgroundUpdates
and a value oftrue
. - Click
Update Release
- Refresh the update URL above; it should now include
disableBackgroundUpdates
@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:
- I can't seem to get localhost:9000 port to work - more notes on that in our relounge channel.
- 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 beboolean
withenum: true
, same asdisableBTS
?
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:
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:
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 samefields
- 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.