Next release

[jmafc]

Benjamin, Daniele,

If @benjamin-kirkbride is successful in implementing the "non-special-casing of extensions" suggested by and with the guidance of @dvarrazzo, I think it would have to be distributed as a new release, say, Pyrseas 0.10, rather than 0.9.2. Therefore, I'd like to discuss other items that would be needed for us to do that. I believe there is at least one must do item and some others that may be desirable and need discussion:

• CI: Before moving much further, we ought to either reinstate the Travis-CI builds or use GH Actions to do builds/tests. I tried to re-enable Travis a while back and wasn't successul (perhaps I didn't try hard enough), but as I understand Actions may provide very similar capabilities (and even additional platforms).
• Folding pgdbconn: I think the code in pgdbconn should be reinstated into Pyrseas. I'd be willing to undertake this step.
• psycopg: Should we move to psycopg3? If so, do we need to continue to support psycopg2? I'm going to have to defer to Daniele's recommendations on this.
• Supported PG/Python: Similarly, should we drop support for Python 2? And what base release of Postgres should be supported? Currently PGDG only supports from PG 10 on. Of course, we could leave all code as is (but docs ought to be updated), but it may be preferable to work on a cleaned-up code base.
• Documentation cleanup.

There may be other items, but I think we should discuss and resolve what is to be done on these.

[benjamin-kirkbride]

Here are my opinions, with the caveat that I realize I am not nearly senior enough in this project for them to carry much weight:

• psycopg3 only supports Python 3, and a relatively recent version of it at that (IIRC) so we will have to drop Py2 if we want to do that
• speaking of dropping Py2, I think that we absolutely should (irrespective of if we move to Psycopg3). I agree there is not a strong imperative to immediately make backwards incompatible changes as a part of this though.
• A couple thoughts on the docs:
  1. more specific documentation on how to test, like step by step instructions
  2. move the API reference to a separate folder/submenu (not sure about the nomenclature), as it is now the sidebar is cluttered by what ultimately is irrelevant to anyone who is not intending to develop against Pyrseas
  3. add some opinionated usage examples, like step by step walk someone who has a blank PG DB through creating an initial schema with yaml2db, pushing it live, modifying the yaml, pushing the diff, then modifying the DDL directly in PG, using db2yaml, etc etc using git throughout. Really show off the utility, workflow, etc to someone without them having to try it out for themselves.

A separate note, how do we feel about using black and isort on the codebase? I realize that code formatting can be a touchy subject, but personally I see a lot of value in using an opinionated autoformatter.

[jmafc]

Regarding documentation cleanup/reorganization, I just rediscovered who had started that in issue #233 (which coincidentally, Benjamin, is about Docker support): @xclayl. His fork is 26 commits ahead of master, but they stop in 2018. I'll have to compare the fork to our docsreorg branch and then decide how best to use/merge them. @benjamin-kirkbride Since you have more experience with Docker, if you have time, please look at that Docker tool and let me know what you think about it.

[benjamin-kirkbride]

I will investigate the Docker stuff tonight.

[dvarrazzo]

While a Docker image is not a bad idea (and not hard to put together) I have read about pipx as a way to install a python "program" and dependencies too.

Pyrseas is pure python, it doesn't depend on other system libraries, right?

[jmafc]

Well it depends on psycopg, which depends on libpq. But I thought that the utility of the tool described in #233 is not for the sake of running Pyrseas but rather installing it and Postgres in a Docker container so that they can be used together.

[benjamin-kirkbride]

Yeah IMO the benefit of a Dockerized Pyrseas is to make it easier to add to CI, things like that.

[dvarrazzo]

• CI: Before moving much further, we ought to either reinstate the Travis-CI builds or use GH Actions to do builds/tests. I tried to re-enable Travis a while back and wasn't successul (perhaps I didn't try hard enough), but as I understand Actions may provide very similar capabilities (and even additional platforms).

I moved pretty much all my free-software workflows from Travis to Github Actions. I advise you to do so.

• psycopg: Should we move to psycopg3? If so, do we need to continue to support psycopg2? I'm going to have to defer to Daniele's recommendations on this.

If you want to do that I would support the choice. I think the main help would come from a better specification of the dependencies: there isn't the mess of the psycopg2 and psycopg2-binary packages both providing the psycopg2 module, stomping on each other's feet. It's the main reason why PGCLI people are migrating, for example (dbcli/pgcli#1085).

• Supported PG/Python: Similarly, should we drop support for Python 2? And what base release of Postgres should be supported?

I think there might be value in supporting certain unsupported versions of Postgres, especially because Pyrseas could be used to migrate away from them. You might even support only supported version and sell your expertise and customize the tool for older versions if they offer you $$$ to do so.

I don't think there's any value in supporting outdated versions of Python. Pyrseas is a command-line tool, not a library that should be part of larger frameworks with a longer lifetime and which is difficult to upgrade or kill. Surely using 3.6 as minimum version would be extremely useful (because of the f-strings). The oldest supported version is 3.7.

[jmafc]

OK, I've created issues #249 and #250. As part of the latter, I'll start removing Py2 code that I find (most or all of it is in handled by lib/pycompat.py). Once pgdbconn is folded back in, we can discuss in more detail how to introduce support for psycopg3 while keeping psycopg2 around.

[dvarrazzo]

If you don't want to change the code we need a row factory generating objects more similar to the DictRow: a tuple with key access rather than a dict with added positional indexes.

I can try and put something together later.

[jmafc]

Ok, now I understand why the behavior changed: the dict_row factory produces results in a different form than the DictConnection used to and that's why the "unpacking" worked nicely before. I don't want to bother with a special row factory to retain the old behavior since my few changes took care of the problems.

[dvarrazzo]

This row factory should maintain the DictConnection semantics. The object returned is a subclass of tuple, so it support unpacking, indexing, iteration, but it also supports access by name.

https://gist.github.com/dvarrazzo/9c874e212406150a1acd8f657a4d914b

[jmafc]

FWIW, the suggested replacement for copy_expert doesn't work because the data is of type memoryview and write() wants a str type. So I'll have to read the docs ...

[jmafc]

Well, I read the docs but it seems to me the previous behavior is unavailable.

I'm using tests/dbobject/test_static.py::StaticTableToMapTestCase::test_copy_static_table_pk to try the suggested code out. The SQL statement inside of lib/dbconn.py::DbConnection::copy_to is

COPY (SELECT * FROM sd.t1 ORDER BY c2, c1) TO STDOUT WITH CSV

As mentioned, if I use for data in copy:, the write() complains that TypeError: write() argument must be str, not memoryview. If I change to for data in copy.rows(): it eventually hits the following psycopg.DataError: bad copy format: got a newline before the end of the row.

If I understand correctly, it looks like in order to use copy.rows() or copy.read_row(), it would be necessary to first specify the format for the items coming in from the COPY, which may be possible, but quite complicated.

Reading further the documentation, I found that "you must not specify COPY options such as FORMAT CSV ...". So I changed pyrseas/dbobject/table.py::Table::data_export so that it would generate the SQL without the WITH CSV. Using straight copy, it still has the memoryview problem with write(). Using copy.rows() fails because the data comes in as a tuple, e.g., the first row in TABLE_DATA2 is returned as ('1', 'abc', 'row 1'), and write() doesn't want tuples either. Finally, using copy.read_row() manages to write the file, but (since I didn't change anything else), it writes fields without any separators, and the backend of the test fails because it finds not enough values to unpack.

If there is no way to retrieve COPY rows in CSV format, the only promising avenue appears to be using copy.rows() together with some CSV formatting capability. I'm not up to doing this (at least not at this time anyway), so unless you have some other suggestions, I'll just disable the tests and note that the "data export" is not currently functional.

[jmafc]

Issue #250 has been taken care of and I have also removed Python 2-related code and documentation. I've also updated the docs for references to PG versions earlier than 10 and some Psycopg URLs that referred to initd.org. Next week I plan to start looking at GH Actions.

[benjamin-kirkbride]

I have not gotten time to do much more than a cursory overview, and will be out of town till next week. I will hopefully get more time next week for this. Sorry!

[dvarrazzo]

FWIW, the suggested replacement for copy_expert doesn't work because the data is of type memoryview and write() wants a str type. So I'll have to read the docs ...

you can use bytes(data) to convert memoryview to bytes, but I thought it would have worked with memoryview too. What psycopg version are you using?

[jmafc]

Psycopg 3.1.1.

If I use bytes, it still fails:

                for data in copy:
>                   f.write(bytes(data))
E                   TypeError: write() argument must be str, not bytes

Writing that with str doesn't help either, because then the test checks what was written and:

            for line in f:
>               (c1, c2, c3) = line.split(',')
E               ValueError: not enough values to unpack (expected 3, got 1)

and FWIW, the file that gets written with str(bytes(data)) is (literally, no newlines anywhere):

b'1\tabc\trow 1\n'b'2\tdef\trow 3\n'b'3\tdef\trow 4\n'b'3\tghi\trow 2\n'

and wrapping str(data) results in a file with <memory at 0x...> repeated n times.

[jmafc]

So, I tried opening the file in binary (which is weird because under Linux, there shouldn't be any difference), but Python3 seems to have created a strange dichotomy between bytes and str. This is the change in dbconn.py copy_to:

+        curs = self.conn.cursor()
+        with curs.copy(sql) as copy:
+            with open(path, "wb") as f:
+                for data in copy:
+                    f.write(bytes(data))

And since it writes tabs instead of commas, I had to change the tests also, .e.g,,

             for line in f:
-                (c1, c2, c3) = line.split(',')
+                (c1, c2, c3) = line.split('\t')

This apparently "fixes" the data export tests, but it also breaks the import tests. Although dbconn.py has a copy_from (it also a copy_to and a sql_copy_to--but I deleted the former), we don't use copy_from but instead append a \copy to read the file in. Aside: I don't recall why we have this asymmetry.

I tried removing the ", csv" from the statements (both in dbobject/table.py and test_static.py). Although the tests run OK if I run them separately (or the two copy tests sequentially and then the two load tests), the tests hang if I run all four of them together, specifically after completing the copy tests.

So we're still not quite out of the woods and even if we manage to fix the tests, I'm quite happy with losing the CSV format because (a) it's much more prevalent, and (b) I'm not sure what PG/psycopg handles a text attribute that has an embedded tab in it (granted, the tests should have verified what PG/psycopg handle an embedded comma).

[jmafc]

Here is the latest status:

With the help of @dvarrazzo, I was able to fix the COPY issues. I also fixed the test_pagila problem (due to another one of those array index accesses). I've run tox and all tests pass on Python3.9 against PG 11 and 13. Except for the obscure operclass tests that are marked xfail, only 8-9 tests are skipped: mostly in test_language, one in external files and one in privileges, which I believe can all be dropped because they would only exercise code prior to PG 9.1.

Next week I plan to remove those tests and any other code that pertains to PG versions earlier than 10.

@benjamin-kirkbride If you plan to do the "non-special casing of extensions" work that Daniele suggested, please let us know. Other than that, in the past two weeks, issue #251 was raised which is actually a duplicate of #175, and which is somewhat related to the extensions issues. I left some comments in #185 because (a) it's the basis for dealing with PG objects we're currently ignoring and (b) the poster of #251 mentioned he was going to check with the "postgres and Python experts" in his team to "find a solution", but that is in limbo too.

If I don't hear from Benjamin or from the people of #251, I plan to release 0.10 as a beta and try to test it against PG 15, so that I can release 0.10 shortly after PG 15 is released.

[benjamin-kirkbride]

@jmafc sorry for the radio silence, life has gotten in the way I'm afraid and it will be at least a month or so before I can circle back to this. I apologize for creating a stir and than dipping, but your efforts are really appreciated!

[jmafc]

Yeah, I almost wrote "life happens" last night and I understand that.

[benjamin-kirkbride]

I did have a thought RE the extensions, not sure if you and/or @dvarrazzo have some reason I'm ignorant of as to why it wouldn't work; if the current plan we have for making extensions function properly doesn't pan out, or only partially solves the problem, what if there was a registry of extensions built in to Pyrseas that would handle the extension specific objects? Hopefully that explanation makes sense

[benjamin-kirkbride]

an example is if the foo extension adds a foo_bar type, that for whatever reason is difficult to detect automatically and/or associate with foo, we just add it to the registry and then Pyrseas knows that when version X of extension foo is present, foo_bar will be too.

[jmafc]

Firstly, I think Daniele's will pan out, the only issue being is that it's like performing surgery on all the major organs of the body and the trick will be in being able to isolate organs, one by one or in groups, so that one can control the overall blood pressure. Second, I have doubts about a "registry". From the Pyrseas perspective, the YAML file(s) and the PG catalogs are the only "registries".

[jmafc]

@dvarrazzo @benjamin-kirkbride
I've run and cleaned up the tests so they all pass against PG 11, 13 and 15, under Python 3.9 on Debian. At this point, I think I could make an official release of Pyrseas 0.10.0. The major enhancement has been moving to Psycopg 3. And of course, support for Python 2.x, as well as Postgres versions earlier than 10, has been dropped. The main hole remains dealing with PG system objects (see #175, #185, #251) and related to it, dealing with extensions (see #209, #236, #244). These most likely will have to wait for a 0.11 release.

I don't think we need to do a beta release. Unless either of you have any comments or other feedback I'll proceed to do the official release early next week.

[benjamin-kirkbride]

Sounds great! Thanks Joe!

[dvarrazzo]

I think that dealing with the extensions will be important, but I agree we can release a 0.10 without :)

[jmafc]

FWIW, I did release 0.10 on GH, PyPi, PGXN and ReadTheDocs. I also sent a mail to pgsql-announce, which was held in moderation. This morning it was rejected because it turns out you now have to post the announcements through a website. I prepared it, but in order to send it I had to first create an "organization", which I did, but that is now held up for moderation ...

[jmafc]

The organization got approved on Tuesday and I posted the news that same evening. Since 36 hours elapsed, I asked the webmasters. Jonathan Katz replied that the moderators are "all volunteering their time, so there is no guarantee on when it will be approved". Furthermore, because of the release of PG 15.1 (this morning), the post is supposedly held up until at least 13 Nov. Oh, well ...

[dvarrazzo]

Congratulations, @jmafc 🎉🎉🎉