Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Verbatim database read/write #466

Merged
merged 22 commits into from
Nov 5, 2019
Merged

Conversation

ederag
Copy link
Collaborator

@ederag ederag commented Oct 30, 2019

With this PR database read/write is done verbatim, without any parsing involved.
This makes a clear separation of concerns.
It partially solves #280 and #423,
although that requires renouncing to the quickness of the cmdline input, until #465 is solved.

This change should not affect the existing dbus interface (old functions have been kept).
The new AddFactVerbatim is strongly encouraged if the caller has separate fields.
The old AddFact will remain so that passing a single string will remain possible,
to be interpreted by Fact.parse on the hamster side.

Unsorted (no category) facts are not supported yet (forthcoming PR almost ready).

@ederag ederag added this to the v3.0 milestone Oct 30, 2019
@ederag ederag mentioned this pull request Oct 30, 2019
4 tasks
@GeraldJansen
Copy link
Contributor

I fetched and checked out ederag/verbatim-db, 68c7381, and did this:

[src]$ pkill -ef hamster.*service
[src]$ ./hamster-cli track a long, hard furrow
Traceback (most recent call last):
  File "./hamster-cli", line 418, in <module>
    getattr(hamster_client, command)(*args.action_args)
  File "./hamster-cli", line 189, in track
    self.start(*args)
  File "./hamster-cli", line 204, in start
    end_time = end_time))
  File "/home/gjansen/proj/hamster/src/hamster/client.py", line 156, in add_fact
    new_id = self.conn.AddFactVerbatim(dbus_fact)
  File "/usr/lib/python3/dist-packages/dbus/proxies.py", line 70, in __call__
    return self._proxy_method(*args, **keywords)
  File "/usr/lib/python3/dist-packages/dbus/proxies.py", line 145, in __call__
    **keywords)
  File "/usr/lib/python3/dist-packages/dbus/connection.py", line 651, in call_blocking
    message, timeout)
dbus.exceptions.DBusException: org.freedesktop.DBus.Error.UnknownMethod: Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/dbus/service.py", line 654, in _message_cb
    (candidate_method, parent_method) = _method_lookup(self, method_name, interface_name)
  File "/usr/lib/python3/dist-packages/dbus/service.py", line 246, in _method_lookup
    raise UnknownMethodException('%s is not a valid method of interface %s' % (method_name, dbus_interface))
dbus.exceptions.UnknownMethodException: org.freedesktop.DBus.Error.UnknownMethod: Unknown method: AddFactVerbatim is not a valid method of interface org.gnome.Hamster

Same error when saving after trying the same input from the edit_activity window, activity field. Giving the same input on the cmdline continues to move the part after the comma to the description field as in #280. At this point I must admit that I'm somewhat confused.

@ederag
Copy link
Collaborator Author

ederag commented Oct 31, 2019

@GeraldJansen Could it be that hamster-service from a previous version is installed ?
In this case, launching hamster-cli queries dbus through client.py,
that automatically relaunches the installed hamster-service,
which lacks the new AddFactVerbatim method.

Could you try to launch the new hamster-service first ?

pkill -ef hamster.*service
./hamster-service
./hamster-cli track a long, hard furrow

If a service is already found running, it is used.

@ederag
Copy link
Collaborator Author

ederag commented Oct 31, 2019

By the way, using your example gives an "a" in activity, the rest is lost.
But that was already the case previously (checked with 5f782f6).
Fact.parse will be reworked to make it usable for command line parsing, after v3.0.

@GeraldJansen
Copy link
Contributor

Could it be that hamster-service from a previous version is installed ?

Right, of course. Sorry for the noise.

OT. Now getting this error message every minute on update:
hamster.lib.desktop - ERROR - Error while refreshing: 'Fact' object is not subscriptable

On a plus note, I don't find any noticeable slowdown in auto-completion from the cmdline. Its still very snappy for me.

@ederag
Copy link
Collaborator Author

ederag commented Oct 31, 2019

Thanks, this one slipped under the radar.
1aefbba is a blind guess. Is it silent now ?
desktop.py is used in hamster-service, but I did not see the error. Strange.

@GeraldJansen
Copy link
Contributor

GeraldJansen commented Oct 31, 2019

1aefbba is a blind guess. Is it silent now ?

Yes. Good guess.

@GeraldJansen
Copy link
Contributor

A small bug (edge case?): if I update an activity that has a category, clear the category and save, the saved fact still has the old category, not Unsorted.

@ederag
Copy link
Collaborator Author

ederag commented Oct 31, 2019

Indeed, that's because after clear the fact has no category.

Unsorted (no category) facts are not supported yet (forthcoming PR almost ready).

That needs a 3 lines only fix, ready, but unfortunately it is slightly breaking the dbus interface:
GetFact and GetFacts would return an empty string instead of "Unsorted".
You may try this fix-unsorted experimental branch,
but please do not base anything on it, as more rebase are expected.
What do you think ?

@GeraldJansen
Copy link
Contributor

GeraldJansen commented Oct 31, 2019

No further comment. As an aside, I've always found the @Unsorted presence in the cmdline strange and unsightly. I always assign a real category.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants