chore) Interpreter completion candidates are filtered out of the suggestion list - #5415
chore) Interpreter completion candidates are filtered out of the suggestion list#5415okayhooni wants to merge 3 commits into
Conversation
|
The fix looks right to me. You asked for suggestions on how to cover this, so I had a go at it: voidmatcha@32a1bbdde Broadcasting |
|
@voidmatcha Sorry for the late reply. Thanks for writing this — it's more than I asked for. 🙇 I cherry-picked 32a1bbd onto the branch as-is, keeping you as the author. The two hooks you found are exactly the right seams. I had looked at FilteredList but did not think to reach it through the Worth noting the first case ("should keep interpreter candidates that carry a meta label") fails on the pre-fix code, so it is a genuine regression test rather than a characterisation of current behaviour. |
What is this PR for?
When an interpreter fills in the
metafield of its completion candidates, none of them reach the suggestion list. PressingCtrl+.shows nothing at all, even though the backend answered with valid candidates.completionSupportWithBackenddecides what to keep by asking whether an item carries ametavalue:The intent of ZEPPELIN-3001 (
fe07e5a49) was to hide ace's own local/keyword suggestions once the interpreter has answered, andmetawas used as the marker for "this candidate came from ace". That assumption does not hold — interpreters tag their own candidates withmetaas well.SqlCompletersendsschema,tableandcolumn:So the filter drops the backend's own suggestions along with ace's, and the list ends up empty.
The failure looks intermittent because the guard only lets anything through while
completionListLengthhappens to be0. That variable is declared per paragraph controller but is updated through$rootScope.$broadcast, and it is reset toundefinedat the end of every filter pass — so whether anything shows depends on what the other paragraphs in the note last answered. Interpreters that leavemetaempty are unaffected, which is why this went unnoticed for so long.This PR marks the candidates built by
remoteCompleterwith an explicitfromBackendflag and filters on that instead of onmeta. The original intent is preserved — ace's own suggestions are still hidden once the interpreter has answered — but the interpreter's candidates are always kept.What type of PR is it?
Bug Fix
Todos
fromBackendflagmetaRelated Jira issue (w/ regression) ?
How should this be tested?
No automated test is included: the change lives in the AngularJS notebook UI (
zeppelin-web/src/app/notebook/paragraph/paragraph.controller.js), which has no test harness covering ace's completion pipeline. Suggestions on how to cover it are welcome.Manual steps, using any interpreter that tags
meta— the JDBC family does:jdbcinterpreter to a database that has at least one schema with tables.SELECT * FROM <schema>.and pressCtrl+..tablemeta label, consistently on every press, and ace'slocal/keywordentries are hidden as ZEPPELIN-3001 intended.Ctrl+.five or six times in a row and confirm the list is identical every time.metaempty (for examplepython) and confirm its behavior is unchanged.Before & After
Ctrl+.)tablelabel)Questions:
interpreters that leave
metaempty; only candidates that were previously dropped by mistake nowappear.