Attention is currently required from: dexter, laforge.

Patch set 4:Code-Review -1

View Change

8 comments:

• Commit Message:

  • Patch Set #4, Line 15: a

    remove
    ```suggestion
    This patch adds PostgreSQL support next to the existing CSV
    ```

• File docs/card-key-provider.rst:

  • Patch Set #4, Line 25: CardKeyProviderCsv

    ```suggestion
    to retrieve the key material from a PostgreSQL database (`CardKeyProviderPgsql`).
    ```

  • Patch Set #4, Line 142: ma

    may

  • Patch Set #4, Line 245: Csv

    ```suggestion
    available from within pySim-shell via the `CardKeyProviderPgsql`.
    ```

  • Patch Set #4, Line 293: CardKeyProviderCsv

    ```suggestion
    As soon as the CardKeyProvider finds a line (row) in your CSV file
    ```

• File pySim-shell.py:

  • Patch Set #4, Line 1186: card_key_provider_register(CardKeyProviderPgsql(opts.pqsql, column_keys))

    That means you can use both providers at the same time, right?

• File pySim/card_key_provider.py:

  • Patch Set #4, Line 218:

            column_names = ""
            for f in fields:
                column_names += f.lower() + ", "
            column_names = column_names[:-2]
    

    This can be done with join:
    ```
    column_names = ", ".join([f.lower() for f in fields])
    ```

  • Patch Set #4, Line 229: cur.execute("SELECT column_name FROM information_schema.columns where table_name = '%s';" % t)

    I believe here and in all other cur.execute statements you don't want to use '%". See https://www.psycopg.org/docs/usage.html#sql-injection

    psycopg takes care of escaping etc., just pass the arguments in a tuple as a separate parameter (e.g. `, (t,)` instead of `% t` here)

    That's probably even more important in the import script above!

To view, visit change 41508. To unsubscribe, or for help writing mail filters, visit settings.