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

Ft/config improvements #21

Merged
merged 12 commits into from
Sep 5, 2016
Merged

Ft/config improvements #21

merged 12 commits into from
Sep 5, 2016

Conversation

dtext
Copy link
Member

@dtext dtext commented Sep 3, 2016

This pull request includes improvements to the config file and a framework for storing persistent application data.

The data can already be encrypted, the encryption feature is just not integrated yet. I will create a new issue for this and link it in this pull request later (edit: see #22).

Closes #15, closes #16.

@dtext dtext added this to the Version 0.1 milestone Sep 3, 2016
@dtext dtext mentioned this pull request Sep 3, 2016
3 tasks
@dtext
Copy link
Member Author

dtext commented Sep 3, 2016

@florianluediger: I added your notes in commit 2a0f24e

# TODO: encryption
kvstore = KVStore(_name)

# FIXME: There is several problems with this:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

googledrive.py needs to be updated before it can make use of the new feature.
As it stands right now, it is very outdated, as explained in the code comments.

@florianluediger
Copy link
Collaborator

Without having looked to deep into the code, I noticed the following when starting the program the first time without an instantshare.conf file (so it has to be automatically created):

The program works just fine but the console output only shows ERROR:root:. All the usual output is missing, the program itself however works just fine. This error only comes up when there is no instantshare.conf file before startup.

error

@dtext
Copy link
Member Author

dtext commented Sep 3, 2016

I encountered that once before, but wasn't sure what happened, so I haven't been able to reproduce it. Thanks, I will look into that.

@dtext
Copy link
Member Author

dtext commented Sep 3, 2016

It seems to be a problem with instantshare.py and the way the config parser is setup.
The problem is that we import CONFIG (which is an instance of Config) before setting up logging, so when the file is missing, the error message is displayed before logging is set up.

Edit: as such, the issue already existed before this pull request, so we should add a bug on the issue tracker (#23).

@florianluediger
Copy link
Collaborator

When using the KVStore the first time without creating a data directory first, the program exits with an error.

--> implement autocreation of the data directory.

@dtext
Copy link
Member Author

dtext commented Sep 3, 2016

Will merge this evening or tomorrow if there are no major objections.


def sync(self):
data = {}
data.update(self)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any reason to copy the data of the dict at this point?

bdata = pickle.dumps(self) should work too.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, there is: I did not want to save an instance of KVStore, as that would include all the other attributes. It's just a bit cleaner, and if we end up changing KVStore in the future, we won't break serialization.

@denniseffing
Copy link
Member

PR looks fine to me, added one minor line comment.

👍

@dtext dtext merged commit eb05e73 into master Sep 5, 2016
@florianluediger florianluediger deleted the ft/config_improvements branch April 20, 2017 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Set a default value for the screenshot_tool variable Better handling of config file and persistent data
3 participants