search

BenMatteson / CS410_Agile_Group2

CS410 Agile Practice Project - FTP Client
0 stars 2 forks source link

Connection working using SSH keys #36

Closed eddiekelleypdx closed 5 years ago

eddiekelleypdx commented 5 years ago

Configured pysftp authentication using SSH keys; Created is_connected() method to get connection status (used for debugging, but potentially other stuff - YAGNI?); Added shebang to allow for script execution Added verbose logging Added checks to confirm whether to use id_rsa and known_hosts Closes #1

Acceptance Criteria:

BenMatteson commented 5 years ago
  1. PSU_ID, etc. makes some assumptions about the user/server/etc. that I don't think we want.
  2. if you made it parse a text file (or csv/json/etc.) it'd be basically doing the used saved credentials task, so I think you're really just over reaching the given task, which while great, isn't really agile. (you could always pick up the task though. :P just make sure you update the tags on the pull to reflect testing for multiple tasks.)

I'll actually look at it closer later if nobody picks up the task before I get to it, I just glanced at it for now. edit - also I added the acceptance criteria to your request.

eddiekelleypdx commented 5 years ago

I was under the impression that the unit tests would only be used by developers of the application (i.e., PSU students in our agile group), but not end users? If unit testing needs to be extended to be a feature provided to end users, making the authentication credentials placeholder more generic might make sense.

The current strategy uses a 3-line python script / text file to store credentials; I'm not sure how much simpler we can make it. This is not meant to implement/replace the saved user credentials feature - it's just the authentication information used for unit testing.

BenMatteson commented 5 years ago

oh, I misread where that code was, that's what I get for glancing at it. My instinct would still be to parse them from some other kind of file, 'cause if we want to get technical I think importing them as constants would keep them in memory (at least in a compiled language, I don't know if python cares?) which is a waste (even if it's just for the unit tests). it's definitely pedantic at this point though, so I don't actually think you need to change it

ppdom commented 5 years ago

The shebang fails on my system (Ubuntu 18.04). I suggest replacing it with #!/usr/bin/env python3 since that is more portable (works for me).

Source: https://stackoverflow.com/a/19305076

BenMatteson commented 5 years ago

This has definitely gotten off topic, it does what it's supposed to so I'm going to pull it before I move everything around.