fix current indexed revision permalink optimization
Categories
(Webtools :: Searchfox, defect)
Tracking
(Not tracked)
People
(Reporter: asuth, Assigned: asuth)
References
Details
Attachments
(1 file)
I regressed the optimization such that permalinks for the current rev no longer directly display what's generated on disk in https://github.com/mozsearch/mozsearch/pull/548 for bug 1703115 when cleaning up/modernizing the string formatting.
The problem we can see here is that the python f-strings will serialize bytes strings as their "repr" for the default (and also explicit !s) string coercion which is a regression compared to the prior % fmt where fmt was a dictionary that had the same value present. What's in the file:
location ~^/mozilla-central/rev/b'5f1fc7348a9fb1189fe1c2139d68f928f8cfe510'/(?<head_path>.+)$ {
Our head_rev is a bytes strings because subprocess.check_output explicitly returns bytes but:
This behaviour may be overridden by setting text, encoding, errors, or universal_newlines to True as described in Frequently Used Arguments and run().
We'll pass text=True and it seems like this will fall back to the preferred encoding which is likely good enough.
At a "what languages/runtimes do we use in searchfox meta level", this is probably an indication that we should continue to move away from Python and towards rust and an explicit templating engine (we've adopted https://crates.io/crates/liquid) for templating related concerns. It's great that Python 3 improved clarity around string types but arguably we're better served by the clarity afforded to us by rust and experiencing failures at compile time under normal idiomatic usage.
I'm going to see about adding a check for this, but one of the problems we have is that the "tests" config is not configured to have a "git_path" because the root wouldn't align. The "searchfox" config, however, does have this, but so far has not been used for checks, so I may end up just validating this fix by inspection.
|
Assignee | |
Comment 1•2 years ago
|
||
The time I was going to spend on the check got gobbled up by investigating the time traveling searchfox situation we experienced today, so I just validated it by inspection. The fix commit is https://github.com/mozsearch/mozsearch/pull/564/commits/60f4500fbf85bd132240b1233c7e349ddf00d2ce landed as an aggregated WIP push at https://github.com/mozsearch/mozsearch/pull/564.
I'll manually validate this tomorrow too but this was a straightforward problem and a straightforward fix.
|
|
Assignee | |
Comment 2•2 years ago
|
||
Quoting the commit message:
The permalink optimization may actually have been broken even longer
than expected as by fixing the route, I revealed that we hadn't
compensated for the gzip optimization I'd done a long, long, long time
ago and so we were serving the 0-length files. This implies the
format string might actually have had the same behavior for the byte
string and that this was broken since the change to python3.
This suggests we probably want a check both for the optimization directly as well as a check against the generated nginx script. Both face the logistical issue of the changing revision identifier but this thankfully is something that the searchfox-tool commands can know as a built-in normalization.
Description
•