Skip to content

feat: remove paramiko#1606

Merged
real-yfprojects merged 19 commits into
borgbase:masterfrom
diivi:feat/remove-paramiko
Mar 17, 2023
Merged

feat: remove paramiko#1606
real-yfprojects merged 19 commits into
borgbase:masterfrom
diivi:feat/remove-paramiko

Conversation

@diivi
Copy link
Copy Markdown
Contributor

@diivi diivi commented Feb 22, 2023

Description

Removed paramiko as a dependency and replaced it with a simple function that checks the first line of a file to verify if it's a ssh key.

Related Issue

#1559

Motivation and Context

Helped remove a heavy dependecy used just for checking if a file is a SSH key.

How Has This Been Tested?

Checked the combobox options

Screenshots (if appropriate):

image

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING guide.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

I provide my contribution under the terms of the license of this repository and I affirm the Developer Certificate of Origin.

Comment thread src/vorta/utils.py Outdated
@diivi diivi marked this pull request as ready for review February 22, 2023 19:48
Comment thread src/vorta/utils.py Outdated
Comment thread src/vorta/utils.py Outdated
Comment thread src/vorta/utils.py Outdated
Comment thread src/vorta/utils.py Outdated
Comment thread src/vorta/utils.py Outdated
Comment thread src/vorta/utils.py Outdated
@diivi diivi requested review from m3nu and real-yfprojects and removed request for m3nu and real-yfprojects February 23, 2023 19:42
Comment thread src/vorta/utils.py Outdated
Comment thread src/vorta/utils.py Outdated
Comment thread src/vorta/utils.py Outdated
Comment thread src/vorta/utils.py Outdated
@diivi diivi requested a review from m3nu February 25, 2023 10:04
Copy link
Copy Markdown
Collaborator

@real-yfprojects real-yfprojects left a comment

Choose a reason for hiding this comment

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

Almost forgot: This PR introduced some clutter in the form on line break removals. I can fix that during merging though.

@diivi
Copy link
Copy Markdown
Contributor Author

diivi commented Feb 25, 2023

Yeah, the pre-commit hooks made some deletions after my changes 😅

Comment thread src/vorta/utils.py Outdated
Comment thread src/vorta/utils.py Outdated
Comment thread src/vorta/utils.py Outdated
Comment thread src/vorta/utils.py Outdated
Copy link
Copy Markdown
Collaborator

@ThomasWaldmann ThomasWaldmann left a comment

Choose a reason for hiding this comment

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

minor consistency change

Comment thread src/vorta/utils.py Outdated
m3nu
m3nu previously approved these changes Mar 9, 2023
Copy link
Copy Markdown
Contributor

@m3nu m3nu left a comment

Choose a reason for hiding this comment

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

Looks good. It's not a big shiny feature, but an important incremental improvement and refactoring.

@diivi diivi requested review from m3nu and removed request for ThomasWaldmann March 16, 2023 21:10
ThomasWaldmann
ThomasWaldmann previously approved these changes Mar 16, 2023
Copy link
Copy Markdown
Collaborator

@ThomasWaldmann ThomasWaldmann left a comment

Choose a reason for hiding this comment

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

LGTM.

m3nu
m3nu previously approved these changes Mar 16, 2023
@m3nu
Copy link
Copy Markdown
Contributor

m3nu commented Mar 16, 2023

Thanks for your patience.

You wanna merge this, @real-yfprojects ? There was some note on formatting you added. Also needs squashing.

@real-yfprojects real-yfprojects dismissed stale reviews from m3nu and ThomasWaldmann via 9fdee76 March 17, 2023 14:47
@real-yfprojects real-yfprojects merged commit 1f12782 into borgbase:master Mar 17, 2023
@diivi diivi deleted the feat/remove-paramiko branch March 17, 2023 15:04
@real-yfprojects real-yfprojects linked an issue Mar 21, 2023 that may be closed by this pull request
diivi added a commit to diivi/vorta that referenced this pull request Apr 11, 2023
Paramiko is a encryption key parsing library. It was used for determining which ssh keys are available on the system. This removes that fairly heavy dependency at replaces it with a very basic heuristic to determine ssh key file by their first line containing `-----BEGIN(\s\w+)? PRIVATE KEY-----`.

* src/vorta/utils.py: Implement `is_ssh_private_key_file`.

* src/vorta/utils.py (get_private_keys): Use `is_ssh_private_key_file` instead of paramiko. Enforce `077` permissions on key files.

* src/vorta/views/ssh_dialog.py : Remove paramiko.

* src/vorta/views/repo_tab.py (RepoTab.init_ssh): Show filename only in `sshComboBox`.

* src/vorta/views/repo_add_dialog.py (AddRepoWindow.init_ssh_key): Show filename only in `sshComboBox`.
DaffyTheDuck pushed a commit to DaffyTheDuck/vorta that referenced this pull request Jun 14, 2023
Paramiko is a encryption key parsing library. It was used for determining which ssh keys are available on the system. This removes that fairly heavy dependency at replaces it with a very basic heuristic to determine ssh key file by their first line containing `-----BEGIN(\s\w+)? PRIVATE KEY-----`.

* src/vorta/utils.py: Implement `is_ssh_private_key_file`.

* src/vorta/utils.py (get_private_keys): Use `is_ssh_private_key_file` instead of paramiko. Enforce `077` permissions on key files.

* src/vorta/views/ssh_dialog.py : Remove paramiko.

* src/vorta/views/repo_tab.py (RepoTab.init_ssh): Show filename only in `sshComboBox`.

* src/vorta/views/repo_add_dialog.py (AddRepoWindow.init_ssh_key): Show filename only in `sshComboBox`.
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.

Remove Paramiko Dependency

5 participants