[Tile-serving] [openstreetmap/osm2pgsql] Add support for osm2pgsql_properties table to osm2pgsql-replication (PR #2029)

[Jochen Topf]

@joto requested changes on this pull request.



> +    def __init__(self, errno, msg):
+        self.errno = errno
+        self.msg = msg
+
+
+class DBConnection:
+
+    def __init__(self, args):
+        self.schema = args.middle_schema
+
+        # If dbname looks like a conninfo string use it as such
+        if args.database and any(part in args.database for part in ['=', '://']):
+            self.conn = psycopg.connect(args.database,
+                                        fallback_application_name="osm2pgsql-replication")
+
+        self.conn = psycopg.connect(dbname=args.database, user=args.username,

This looks strange. Should this be in an `else:` block?

> +        self.conn = psycopg.connect(dbname=args.database, user=args.username,
+                                    host=args.host, port=args.port,
+                                    fallback_application_name="osm2pgsql-replication")
+
+        self.name = self.conn.get_dsn_parameters()['dbname']
+
+    def __enter__(self):
+        return self
+
+    def __exit__(self, exc_type, exc_value, traceback):
+        if self.conn is not None:
+            self.conn.close()
+
+    def table_exists(self, table_name):
+        with self.conn.cursor() as cur:
+            cur.execute('SELECT * FROM pg_tables where tablename = %s and schemaname = %s ',

Inconsistent capitalization.

>          Given the OSM data
             """
             n34 Tamenity=restaurant x77 y45.3
             """
+        And the replication service at http://example.com/europe/liechtenstein-updates
         When running osm2pgsql pgsql with parameters
             | --slim |
 
         And running osm2pgsql-replication
             | init | --osm-file={TEST_DATA_DIR}/liechtenstein-2013-08-03.osm.pbf |
 

Its confusing that osm2pgsql is run with different input than osm2pgsql-replication? Maybe that warrents a comment?

> +        return self
+
+
+    def get_state_info(self, seq=None, retries=2):
+        assert self.state_infos, 'Replication mock not propoerly set up'
+        if seq is None:
+            return self.state_infos[-1]
+
+        for info in self.state_infos:
+            if info.sequence == seq:
+                return info
+
+        assert False, f"No sequence information for sequence ID {seq}."
+
+    def timestamp_to_sequence(self, timestamp, balanced_search=False):
+        assert self.state_infos, 'Replication mock not propoerly set up'

`propoerly` typo

> +
+class ReplicationServerMock:
+
+    def __init__(self):
+        self.expected_base_url = None
+        self.state_infos = []
+
+
+    def __call__(self, base_url):
+        assert self.expected_base_url is not None and base_url == self.expected_base_url,\
+               f"Wrong replication service called. Expected '{self.expected_base_url}', got '{base_url}'"
+        return self
+
+
+    def get_state_info(self, seq=None, retries=2):
+        assert self.state_infos, 'Replication mock not propoerly set up'

`propoerly` typo

> @@ -89,6 +93,14 @@ def before_all(context):
     context.test_data_dir = Path(context.config.userdata['TEST_DATA_DIR']).resolve()
     context.default_data_dir = Path(context.config.userdata['SRC_DIR']).resolve()
 
+    # Set up replication script.
+    replicationfile = str(Path(context.config.userdata['REPLICATION_SCRIPT']).resolve())
+    spec = importlib.util.spec_from_loader('osm2pgsql_replication',
+                                           SourceFileLoader( 'osm2pgsql_replication',replicationfile))

Space character in wrong place. :-)

>          When running osm2pgsql pgsql with parameters
             | --slim |
 
         And running osm2pgsql-replication
             | init | --osm-file={TEST_DATA_DIR}/liechtenstein-2013-08-03.osm.pbf |
 
-        Then table planet_osm_replication_status has 1 row
+        Then table osm2pgsql_properties contains
+            | property                    | value                                           |
+            | replication_base_url        | http://example.com/europe/liechtenstein-updates |
+            | replication_sequence_number | 9999999                                         |

Where does this sequence number come from?

> +        Given the input file 'liechtenstein-2013-08-03.osm.pbf'
+        And the replication service at http://example.com/europe/liechtenstein-updates
+        When running osm2pgsql pgsql with parameters
+            | --slim | --middle-schema=foobar |
+
+        And running osm2pgsql-replication
+            | init | --middle-schema=foobar |
+
+        Then table foobar.osm2pgsql_properties contains
+            | property                    | value                                           |
+            | replication_base_url        | http://example.com/europe/liechtenstein-updates |
+            | replication_sequence_number | 9999999                                         |
+            | replication_timestamp       | 2013-08-03T19:00:02Z                            |
+
+
+    Scenario: Replication initialiasion will fail for a database in a different schema

`initialiasion` typo

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/osm2pgsql/pull/2029#pullrequestreview-1571795380
You are receiving this because you are subscribed to this thread.

Message ID: <openstreetmap/osm2pgsql/pull/2029/review/1571795380 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/tile-serving/attachments/20230810/690ae10c/attachment.htm>