[Tile-serving] [openstreetmap/osm2pgsql] add an 'info' command which prints the lag (& in JSON) (#1548)

[Sarah Hoffmann]

@lonvia requested changes on this pull request.

This is okay in principle but I would like to discuss the JSON output. I find the duplicated information in the age fields rather strange. If you have a computer-readable format, then providing the computer-readable output should be enough. Pretty-printing should be done on the client side. So I would just do `age: <age in seconds>`. Same goes for the `diff` section. It's trivial for a client to subtract sequence numbers or age seconds. No need to duplicate that on the server side.

>  .SH DISTRIBUTION
-The latest version of osm2pgsql\-replication may be downloaded from
+The latest version of <<UNSET \-\-project\-name OPTION>> may be downloaded from

As per other comment, please revert those four lines.

> +            LOG.fatal(results['error']['text']['en'])
+            return results['error']['code']
+
+        LOG.info("Using replication service '%s'. Current sequence %d (%s).",
+                 results['server']['base_url'], results['local']['sequence'], results['local']['timestamp'])
+        LOG.info("Server is at sequence %d (%s)", results['server']['sequence'], results['server']['timestamp'])
+        LOG.info("Server's most recent data is %s old", results['server']['age']['text']['en'])
+
+        if results['local']['sequence'] == results['server']['sequence']:
+            LOG.info("Database is up to date with server")
+        else:
+            LOG.info("Database is %d sequences behind, i.e. %s",
+                    results['diff']['sequence'],
+                    results['diff']['timestamp']['text']['en'])
+
+        LOG.info("Database's most recent data is %s old", results['local']['age']['text']['en'])

This is where moving log output to stdout gets you into trouble. The status should be printed to stdout while all other log output needs to go to stderr. So use print here as for the json dump.

> +            if cur.rowcount != 1:
+                results['error'] = {
+                        'text': {'en': "Updates not set up correctly. Run 'osm2pgsql-updates init' first."},
+                        'code': (1 << 1)
+                        }
+            else:
+
+                base_url, db_seq, db_ts = cur.fetchone()
+                db_ts = db_ts.astimezone(dt.timezone.utc)
+                results['server']['base_url'] = base_url
+                results['local']['sequence'] = db_seq
+                results['local']['timestamp'] = db_ts.strftime("%Y-%m-%dT%H:%M:%SZ")
+
+
+                repl = ReplicationServer(base_url)
+                server_seq, server_ts = repl.get_state_info()

Check for a return value of None here which you get when the server could not be reached.

> @@ -115,6 +134,84 @@ def update_replication_state(conn, table, seq, date):
 
     conn.commit()
 
+def status(conn, args):
+    """\
+    Print information about the current replication status
+    """

I could swear the initial version had more documentation in the man page. Extended documentation (including a description of the json output should be added here and will then automatically appear in the man page.

Also: please finish the sentence with a full stop.

> @@ -340,6 +437,16 @@ def get_parser():
     cmd.add_argument('--post-processing', metavar='SCRIPT',
                      help='Post-processing script to run after each execution of osm2pgsql.')
 
+    # Arguments for status
+    cmd = subs.add_parser('status', parents=[default_args],
+                          help=status.__doc__.split('\n', 1)[0],
+                          description=dedent(status.__doc__),
+                          formatter_class=RawDescriptionHelpFormatter,
+                          add_help=False)
+    cmd.add_argument('--json', action="store_true", default=False, help="Output status as json")

Full stop missing in help text.

> +def pretty_format_timedelta(delta):
+    minutes = int(delta.total_seconds()/60)
+    (hours, minutes) = divmod(minutes, 60)
+    (days, hours) = divmod(hours, 24)
+    (weeks, days) = divmod(days, 7)
+
+    output = []
+    # If weeks > 1 but hours == 0, we still want to show "0 hours"
+    if weeks > 0:
+        output.append("{} week(s)".format(weeks))
+    if days > 0 or weeks > 0:
+        output.append("{} day(s)".format(days))
+    if hours > 0 or days > 0 or weeks > 0:
+        output.append("{} hour(s)".format(hours))
+
+    output.append("{} minutes(s)".format(minutes))

minutes -> minute

> +
+
+                repl = ReplicationServer(base_url)
+                server_seq, server_ts = repl.get_state_info()
+                server_ts = server_ts.astimezone(dt.timezone.utc)
+
+                results['server']['sequence'] = server_seq
+
+                results['server']['timestamp'] = server_ts.strftime("%Y-%m-%dT%H:%M:%SZ")
+                server_age = dt.datetime.now(dt.timezone.utc) - server_ts
+                results['server']['age'] = {}
+                results['server']['age']['seconds'] = int(server_age.total_seconds())
+                results['server']['age']['text'] = {'en': pretty_format_timedelta(server_age) }
+
+                results['local']['age'] = {}
+                local_age = dt.datetime.now(dt.timezone.utc) - db_ts

`now()` should be called only once or you might end up with odd discrepancies for the age.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/osm2pgsql/pull/1548#pullrequestreview-728724735
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/tile-serving/attachments/20210812/87083ff8/attachment-0001.htm>