Attention is currently required from: daniel, fixeria, laforge.

dexter has posted comments on this change by dexter. ( 
https://gerrit.osmocom.org/c/pysim/+/41508?usp=email )

Change subject: card_key_provider: add PostgreSQL support
......................................................................


Patch Set 5:

(13 comments)

Commit Message:

https://gerrit.osmocom.org/c/pysim/+/41508/comment/8083cfcd_669b57d1?usp=email :
PS4, Line 15: a
> remove […]
Done


File contrib/csv-to-pgsql.py:

https://gerrit.osmocom.org/c/pysim/+/41508/comment/bc72029d_059cfcd5?usp=email :
PS4, Line 16: # cmd2 >= 2.3.0 has deprecated the bg/fg in favor of Bg/Fg :(
> Other than `YELLOW`, `cmd2` API is not used in this script. […]
ufortunately the API of the PySimLogger only accepts those CMD2 enums. To make 
the use of the PySimLogger easier in stand-alone programs like this I have 
added some flexibility to it (see the patch before this one). Now we can just 
pass an ANSI color code like you suggest.


https://gerrit.osmocom.org/c/pysim/+/41508/comment/56531c3a_9f84d5da?usp=email :
PS4, Line 225: log.info("CSV file: %s" % opts.csv)
> BTW, for logging API it's recommended to use lazy format-sting evaluation: […]
Done


https://gerrit.osmocom.org/c/pysim/+/41508/comment/595274da_852549e8?usp=email :
PS4, Line 227: if not csv_file:
> Does `open()` return `None` at all? It raises an exception on error.
Done


File docs/card-key-provider.rst:

https://gerrit.osmocom.org/c/pysim/+/41508/comment/7fd57bd5_aeef8df7?usp=email :
PS4, Line 25: CardKeyProviderCsv
> ```suggestion […]
Done


https://gerrit.osmocom.org/c/pysim/+/41508/comment/613c6eea_29bcf859?usp=email :
PS4, Line 142: ma
> may
Done


https://gerrit.osmocom.org/c/pysim/+/41508/comment/5a459779_803ddada?usp=email :
PS4, Line 245: Csv
> ```suggestion […]
Done


https://gerrit.osmocom.org/c/pysim/+/41508/comment/225a8913_729f0b94?usp=email :
PS4, Line 293: CardKeyProviderCsv
> ```suggestion […]
Done


File pySim-shell.py:

https://gerrit.osmocom.org/c/pysim/+/41508/comment/93310cc0_a32d38b1?usp=email :
PS4, Line 1186:         
card_key_provider_register(CardKeyProviderPgsql(opts.pqsql, column_keys))
> That means you can use both providers at the same time, right?
Yes, this is intentional. Users may want to use a local CSV file in addition. 
However, one of the two will probably the usual case.


File pySim/card_key_provider.py:

https://gerrit.osmocom.org/c/pysim/+/41508/comment/d87bf4d6_7746f75b?usp=email :
PS4, Line 218:         column_names = ""
             :         for f in fields:
             :             column_names += f.lower() + ", "
             :         column_names = column_names[:-2]
> This can be done with join: […]
Done


https://gerrit.osmocom.org/c/pysim/+/41508/comment/49b3cc87_d865a700?usp=email :
PS4, Line 229:             cur.execute("SELECT column_name FROM 
information_schema.columns where table_name = '%s';" % t)
> yeah. […]
Done


https://gerrit.osmocom.org/c/pysim/+/41508/comment/7d34e1fa_f35aee47?usp=email :
PS4, Line 242: break;
> cosmetic: semicolon is not needed here
Done


File setup.py:

https://gerrit.osmocom.org/c/pysim/+/41508/comment/d7076949_c8c8decc?usp=email :
PS4, Line 37: psycopg2-binary
> Not blocking here, but do we really want this as a required dependency? This 
> list is already quite l […]
What if we leave it out and mention it in the manual that it is needed when 
someone wants to use the CardKeyProviderPgsql? Would that be an option? There 
shouldn't be any problems until someone tries to actually use 
CardKeyProviderPgsql.



--
To view, visit https://gerrit.osmocom.org/c/pysim/+/41508?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings?usp=email

Gerrit-MessageType: comment
Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: Icba625c02a60d7e1f519b506a46bda5ded0537d3
Gerrit-Change-Number: 41508
Gerrit-PatchSet: 5
Gerrit-Owner: dexter <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <[email protected]>
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-CC: fixeria <[email protected]>
Gerrit-Attention: laforge <[email protected]>
Gerrit-Attention: fixeria <[email protected]>
Gerrit-Attention: daniel <[email protected]>
Gerrit-Comment-Date: Thu, 18 Dec 2025 15:03:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <[email protected]>
Comment-In-Reply-To: fixeria <[email protected]>
Comment-In-Reply-To: daniel <[email protected]>

Reply via email to