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

Update pykeepass #10

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

floriankisser
Copy link
Contributor

I couldn't open my db due to libkeepass/pykeepass#152.

@floriankisser
Copy link
Contributor Author

Python 3.4 isn't getting security updates anymore, support was dropped by lxml.

@fopina
Copy link
Owner

fopina commented Nov 23, 2019

Can you put the version pinning back into setup.py?
It’s a sensitive package and I only use it with pinning to avoid possible takeovers (pypi does not allow version replacements)
I’ll merge after, cheers on the PR!

@floriankisser
Copy link
Contributor Author

floriankisser commented Nov 23, 2019

OK, I get why you did it in the first place. But as every dependency can be used for an attack just as easily, wouldn't it be better to pip freeze all dependencies to the requirements.txt and encourage people to install via pip install -r requirements.txt?

@fopina
Copy link
Owner

fopina commented Nov 23, 2019

That’s true, it would be the best. But people never install CLI packages in a virtual env, they do it in system or user python path. Freezing all dependencies breaks a lot of things for other installed apps...
Anyway, better not go down any existential rabbit holes for something like this package :)
Let’s just keep this PR clean for its purpose: upgrade keepass version

  • requirements version
  • setup py version
  • tests for new exception raised

Remove everything else please to keep the focus of the PR so I can merge it :)

@floriankisser
Copy link
Contributor Author

I created another pull request for the other changes.

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