levylabpitt / LV-Data

Collection of LabVIEW VIs to read and write lab data.
BSD 3-Clause "New" or "Revised" License
3 stars 1 forks source link

PG: PGPASS GUI #102

Closed ciozi137 closed 1 year ago

ciozi137 commented 1 year ago

Right now there are no GUI utilities to help the end user populate and manage the PGPASS file. The user is required to know the location of the file, understand its format, etc. There is an existing utility, PGPASS.dialog.vi, that will accept user input, but it is lacking several essential features:

  1. it does not manage dropping duplicate connections
  2. it does not drop user-selected connections
  3. header (# host:port:database:user:password) should remain unchanged
  4. Provide a way to save previous known good connection? Database Viewer has hard-coded user=reader. If this doesn't work (suppose we change reader to llab_reader, how should Database Viewer get save the new connection? If this is an application-level problem, eg. save the change to Database Viewer's config,ini, then the PGPASS library at minimum should return useful information to the calling application)

PGPASS.dialog.vi (Current):

image image

PGPASS.dialog.vi (Potential Fix?):

image

Some of these features (1.) should probably be handled by PGPASS.write.vi

PGPASS.write.vi (Current):

image

PGPASS.write.vi (Potential Fix?):

image

pgwijesinghe commented 1 year ago

How about we create a user manager GUI to manage the PGPASS? I created a dummy VI with following functionalities:

  1. Display the existing users from the PGPASS - Reads the current file and displays user information except the password.
  2. Adding users to the PGPASS - Can add new users to the file. Duplicate usernames are not allowed.
  3. Deleting users from the PGPASS - A user can be deleted from the file given that the correct password is entered.
  4. I've assumed an empty PGPASS file is created w/ software. If not, that too can be handled by the User manager.

I created this primarily with the use case of Database Viewer in mind. I didn't think about the other use cases of LV-Data package. In terms of the Database Viewer, at app startup, we can prompt this user manager so the user can login/manage to a desired account.

Changes made to the existing structure:

Add: PGPASS.UserMgr VI 2023-08-31_15-50-39

Modify: PGPASS.Read VI (Modified to return the password for validation purposes) image

Modify: PGPASS.Write VI (Modified to include the header and the footer) image

ciozi137 commented 1 year ago

@pgwijesinghe this looks great. You can push your changes and I can review them ASAP.

ciozi137 commented 1 year ago

@pgwijesinghe some feedback:

  1. timeout = 0 is not good. The program will use too many resources. Normally 100 ms is ok, 10 ms maybe if you want it to be really fast. For this program -1 is best because we can just respond to user events. image

  2. it is best to have minimal code inside the event case. in this case you are reading the file over and over and over unnecessarily. read it once and store the data. image

  3. this program has already outgrown the architecture you've used and it should be a JKI state machine (which should be the default almost all of the time). Here I'm updating the host/port/... indicators by reading from the data cluster, in a event cluster with timeout=-1, and only in response to the "select user" value change. (also notice you can use OldVal or NewVal data instead of a local variable) image

  4. What you have for Add/Delete Users is a great start. I need to think about the logic a little bit though, because 101.202.30.404:5432:database:reader:password and 505.606.70.808:5432:database:reader:password would be two allowed, non-duplicate entries with the same user name. image

ciozi137 commented 1 year ago

A very real, non-contrived example is:

3.128.20.209:5432:levylab:llab_admin:******
3.128.20.209:5432:levylab_test:llab_admin:******
pgwijesinghe commented 1 year ago

Yeah, I agree. It's not optimized. I just wanted to build a dummy VI to test the functionality and compatibility with our use cases. I'll work on optimizing it with a state machine (I see that you've already built one. would you be able to push the changes to the remote issue branch so that I can start from there?). And yeah, your point makes sense. I'll think about a new deduping logic. It'll be much cleaner with a state machine.

ciozi137 commented 1 year ago

Yes I will push my changes shortly