Fava QueryShell Change

[aclindsa]

Upstream Fava changed the arguments to QueryShell.execute_query() in beancount/fava@6f4bcd5#diff-7ae3e08b8e89a98b64814a30622d443c8b09336df4f7b866c941476f14090bc5 such that it takes an additional positional argument, 'entries', prior to the pre-existing 'query' argument.

This leads to failures like:

Exception on /my-finances/extension/Investor/ [GET]
Traceback (most recent call last):
  File "/usr/local/lib/python3.10/dist-packages/flask/app.py", line 2077, in wsgi_app
    response = self.full_dispatch_request()
  File "/usr/local/lib/python3.10/dist-packages/flask/app.py", line 1525, in full_dispatch_request
    rv = self.handle_user_exception(e)
  File "/usr/local/lib/python3.10/dist-packages/flask/app.py", line 1523, in full_dispatch_request
    rv = self.dispatch_request()
  File "/usr/local/lib/python3.10/dist-packages/flask/app.py", line 1509, in dispatch_request
    return self.ensure_sync(self.view_functions[rule.endpoint])(**req.view_args)
  File "/usr/local/lib/python3.10/dist-packages/fava/application.py", line 345, in extension_report
    content = render_template_string(template, extension=extension)
  File "/usr/local/lib/python3.10/dist-packages/flask/templating.py", line 166, in render_template_string
    return _render(ctx.app.jinja_env.from_string(source), context, ctx.app)
  File "/usr/local/lib/python3.10/dist-packages/flask/templating.py", line 128, in _render
    rv = template.render(context)
  File "/usr/local/lib/python3.10/dist-packages/jinja2/environment.py", line 1301, in render
    self.environment.handle_exception()
  File "/usr/local/lib/python3.10/dist-packages/jinja2/environment.py", line 936, in handle_exception
    raise rewrite_traceback_stack(source=source)
  File "<template>", line 46, in top-level template code
  File "/usr/local/lib/python3.10/dist-packages/fava_investor/__init__.py", line 44, in build_tlh_tables
    return libtlh.get_tables(accapi, self.config.get('tlh', {}))
  File "/usr/local/lib/python3.10/dist-packages/fava_investor/modules/tlh/libtlh.py", line 16, in get_tables
    retrow_types, to_sell, recent_purchases = find_harvestable_lots(accapi, options)
  File "/usr/local/lib/python3.10/dist-packages/fava_investor/modules/tlh/libtlh.py", line 107, in find_harvestable_lots
    rtypes, rrows = accapi.query_func(sql)
  File "/usr/local/lib/python3.10/dist-packages/fava_investor/common/favainvestorapi.py", line 28, in query_func
    contents, rtypes, rrows = self.ledger.query_shell.execute_query(sql)
TypeError: QueryShell.execute_query() missing 1 required positional argument: 'query'

[aclindsa]

I did some shallow digging, and came up with a candidate for a fix for this issue: #70

I've attempted to gate the change based on the fava version, but I confess I have not tested it with earlier versions (fava < 1.22) to ensure it still works.

[redstreet]

Awesome, thanks for catching these bleeding edge issues. I tend to be slow to ugprade versions, so I'm glad you seem to be on the other end :-) 👍

Left a comment on the PR. Once ready, I'll test on (fava < 1.22) before merging.

[aclindsa]

Awesome, thanks for catching these bleeding edge issues. I tend to be slow to ugprade versions, so I'm glad you seem to be on the other end :-) +1

Left a comment on the PR. Once ready, I'll test on (fava < 1.22) before merging.

Sounds good! I'll take a look and address it in the next few days as I can.

And FYI, it appears there is another possible issue that I'm now hitting with the latest upstream fava where the output from fava_investor is being output without error by fava, but as raw, un-rendered HTML. I'm suspecting this is another API change we'll need to work around, but haven't yet made time to 1) confirm its not something that I've broken and 2) triage it. So there may be an additional fix coming.

[aclindsa]

And FYI, it appears there is another possible issue that I'm now hitting with the latest upstream fava where the output from fava_investor is being output without error by fava, but as raw, un-rendered HTML. I'm suspecting this is another API change we'll need to work around, but haven't yet made time to 1) confirm its not something that I've broken and 2) triage it. So there may be an additional fix coming.

It looks like this was an issue in Fava itself: beancount/fava#1440

[redstreet]

BTW: I made this change in 757fc20 (in addition to a03223a) so all modules now respect the UI context. Still waiting on beancount/fava#1467 to complete it.

If you happen to test this out, feedback is appreciated 😃 .