1113475 - Add support for faking extVersion in schema_version 1

Status: RESOLVED FIXED

(Release Engineering Graveyard :: Applications: Balrog (backend), defect)

Description

[Nick Thomas [:nthomas] (UTC+12)]

Bug 998721 requests we try to get really old users up to 12.0, and hopefully onwards. We need to fake the extVersion to be the same as the version in the request, so the user isn't shown Incompatible Extensions dialog.

The code to change is at
 https://github.com/mozilla/balrog/blob/master/auslib/blobs/apprelease.py#L337
and where it's called.

Comment 1

[[e-mail] (:bhearsum)]

Attachment: add support for faking extv in schema 1 blobs

I think this patch addressed this bug. It's not pretty, but I think it's okay for something like this?

Comment 2

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 8562945 [details] [diff] [review]
add support for faking extv in schema 1 blobs

Review of attachment 8562945 [details] [diff] [review]:
-----------------------------------------------------------------

::: auslib/blobs/apprelease.py
@@ +354,5 @@
>  
> +    def createXML(self, updateQuery, *args, **kwargs):
> +        xml = super(ReleaseBlobV1, self).createXML(updateQuery, *args, **kwargs)
> +        if self.get("setExtvToIncomingVersion"):
> +            xml = re.sub('extensionVersion="[\w.]+"', 'extensionVersion="%s"' % updateQuery["version"], xml)

I'd rather see this done in ReleaseBlobV1._getUpdateLineXML() at
  https://github.com/mozilla/balrog/blob/master/auslib/blobs/apprelease.py#L337
to avoid a regexp call. eg changing the value of extv if setExtvToIncomingVersion is True.

The test looks fine.

Comment 3

[[e-mail] (:bhearsum)]

Attachment: schema 1 extv hack without regex

(In reply to Nick Thomas [:nthomas] from comment #2)
> Comment on attachment 8562945 [details] [diff] [review]
> add support for faking extv in schema 1 blobs
> 
> Review of attachment 8562945 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: auslib/blobs/apprelease.py
> @@ +354,5 @@
> >  
> > +    def createXML(self, updateQuery, *args, **kwargs):
> > +        xml = super(ReleaseBlobV1, self).createXML(updateQuery, *args, **kwargs)
> > +        if self.get("setExtvToIncomingVersion"):
> > +            xml = re.sub('extensionVersion="[\w.]+"', 'extensionVersion="%s"' % updateQuery["version"], xml)
> 
> I'd rather see this done in ReleaseBlobV1._getUpdateLineXML() at
>  
> https://github.com/mozilla/balrog/blob/master/auslib/blobs/apprelease.py#L337
> to avoid a regexp call. eg changing the value of extv if
> setExtvToIncomingVersion is True.
> 
> The test looks fine.

I went down that road too, but that method doesn't have access to the incoming extv. I could pass it down (which would mean supporting it as an arg to all such methods, or maybe do something like use ext=%OLDVERSION% and have the overridden createXML sub that - both of those are a bit nasty IMO though.

How about this? It still overrides createXML but I found a way to avoid the regex.

Comment 4

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 8563404 [details] [diff] [review]
schema 1 extv hack without regex

The mega-comment block at https://github.com/mozilla/balrog/blob/master/auslib/blobs/apprelease.py#L100 needs a little tweak.

Comment 5

[[github robot]]

Commit pushed to master at https://github.com/mozilla/balrog

https://github.com/mozilla/balrog/commit/a264bf63954041ebcb8f6352cd77b3692bb0db2c
bug 1113475: Add support for faking extVersion in schema_version 1. r=nthomas

Comment 6

[[e-mail] (:bhearsum)]

Comment on attachment 8563404 [details] [diff] [review]
schema 1 extv hack without regex

Landed with an update to the comment block.

Comment 7

[[e-mail] (:bhearsum)]

This is in production now.