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=ema... : PS4, Line 15: a
remove […]
Done
File contrib/csv-to-pgsql.py:
https://gerrit.osmocom.org/c/pysim/+/41508/comment/bc72029d_059cfcd5?usp=ema... : 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=ema... : 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=ema... : 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=ema... : PS4, Line 25: CardKeyProviderCsv
Done
https://gerrit.osmocom.org/c/pysim/+/41508/comment/613c6eea_29bcf859?usp=ema... : PS4, Line 142: ma
may
Done
https://gerrit.osmocom.org/c/pysim/+/41508/comment/5a459779_803ddada?usp=ema... : PS4, Line 245: Csv
Done
https://gerrit.osmocom.org/c/pysim/+/41508/comment/225a8913_729f0b94?usp=ema... : PS4, Line 293: CardKeyProviderCsv
Done
File pySim-shell.py:
https://gerrit.osmocom.org/c/pysim/+/41508/comment/93310cc0_a32d38b1?usp=ema... : 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=ema... : 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=ema... : 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=ema... : 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=ema... : 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.
-
dexter