Merge branch 'master' into irve-packing

.credo.exs

@@ -129,10 +129,15 @@
 
         # Deprecated checks (these will be deleted after a grace period)
         #
-        {Credo.Check.Readability.Specs, false}
+        {Credo.Check.Readability.Specs, false},
 
         # Custom checks can be created using `mix credo.gen.check`.
         #
+
+        {Credo.Check.Warning.ForbiddenModule,
+         [
+           modules: [Timex]
+         ]}
       ]
     }
   ]

apps/shared/lib/date_time_display.ex

@@ -34,7 +34,7 @@ defmodule Shared.DateTimeDisplay do
   def format_date(nil, _), do: ""
 
   def format_date(date, locale, iso_extended: true) do
-    date |> Timex.parse!("{ISO:Extended}") |> format_date(locale)
+    date |> TimeWrapper.parse!("{ISO:Extended}") |> format_date(locale)
   end
 
   @doc """
@@ -100,7 +100,7 @@ defmodule Shared.DateTimeDisplay do
 
   def format_datetime_to_paris(datetime, locale, options) when is_binary(datetime) do
     datetime
-    |> Timex.parse!("{ISO:Extended}")
+    |> TimeWrapper.parse!("{ISO:Extended}")
     |> format_datetime_to_paris(locale, options)
   end
 
@@ -157,7 +157,7 @@ defmodule Shared.DateTimeDisplay do
 
   def format_time_to_paris(datetime, locale, options) when is_binary(datetime) do
     datetime
-    |> Timex.parse!("{ISO:Extended}")
+    |> TimeWrapper.parse!("{ISO:Extended}")
     |> format_time_to_paris(locale, options)
   end
 
@@ -225,14 +225,11 @@ defmodule Shared.DateTimeDisplay do
 
   @spec convert_to_paris_time(DateTime.t() | NaiveDateTime.t()) :: DateTime.t()
   def convert_to_paris_time(%DateTime{} = dt) do
-    case Timex.Timezone.convert(dt, "Europe/Paris") do
-      %Timex.AmbiguousDateTime{after: dt} -> dt
-      %DateTime{} = dt -> dt
-    end
+    TimeWrapper.convert_to_paris_time(dt)
   end
 
   def convert_to_paris_time(%NaiveDateTime{} = ndt) do
-    ndt |> Timex.Timezone.convert("UTC") |> convert_to_paris_time()
+    ndt |> TimeWrapper.convert("UTC") |> convert_to_paris_time()
   end
 
   defp get_localized_datetime_format("en" = locale, options) do

apps/shared/lib/time_wrapper.ex

@@ -0,0 +1,48 @@
+defmodule TimeWrapper do
+  @moduledoc """
+  This module concentrates all the calls to `Timex` in a single place.
+
+  The idea behind this module is 1. to reduce our dependency on `Timex`, and
+  2. to ideally gradually replace calls by built-in Elixir `DateTime` calls, since
+  `Timex` filled a void in the language that has been partially filled now.
+  """
+
+  # credo:disable-for-this-file Credo.Check.Warning.ForbiddenModule
+
+  def parse!(date_as_string, "{ISO:Extended}" = param) do
+    Timex.parse!(date_as_string, param)
+  end
+
+  def parse!(date_as_string, "{YYYY}{0M}{0D}" = param) do
+    Timex.parse!(date_as_string, param)
+  end
+
+  # NOTE: try not to use this, we will remove it. This is rfc2822 ;
+  # Plug encodes it, but there is no built-in decoder.
+  def parse!(datetime_as_string, "{WDshort}, {D} {Mshort} {YYYY} {h24}:{m}:{s} GMT" = param) do
+    Timex.parse!(datetime_as_string, param)
+  end
+
+  def diff(first, second, :hours = param) do
+    Timex.diff(first, second, param)
+  end
+
+  def now do
+    Timex.now()
+  end
+
+  def shift(dt, months: months) do
+    Timex.shift(dt, months: months)
+  end
+
+  def convert(dt, "UTC") do
+    Timex.Timezone.convert(dt, "UTC")
+  end
+
+  def convert_to_paris_time(dt) do
+    case Timex.Timezone.convert(dt, "Europe/Paris") do
+      %Timex.AmbiguousDateTime{after: dt} -> dt
+      %DateTime{} = dt -> dt
+    end
+  end
+end

apps/shared/test/time_wrapper_test.exs

@@ -0,0 +1,4 @@
+defmodule TimeWrapperTest do
+  use ExUnit.Case, async: true
+  doctest TimeWrapper
+end

apps/transport/lib/db/dataset.ex

@@ -154,7 +154,7 @@ defmodule DB.Dataset do
   @spec type_to_str_map() :: %{binary() => binary()}
   def type_to_str_map,
     do: %{
-      "public-transit" => dgettext("db-dataset", "Public transit - static schedules"),
+      "public-transit" => dgettext("db-dataset", "Public transit"),
       "carpooling-areas" => dgettext("db-dataset", "Carpooling areas"),
       "carpooling-lines" => dgettext("db-dataset", "Carpooling lines"),
       "carpooling-offers" => dgettext("db-dataset", "Carpooling offers"),

apps/transport/lib/irve/extractor.ex

@@ -114,6 +114,7 @@ defmodule Transport.IRVE.Extractor do
     |> Enum.map(fn x ->
       Map.take(x, [
         :dataset_id,
+        :http_status,
         :dataset_title,
         :dataset_organisation_name,
         :dataset_organisation_url,
@@ -136,7 +137,7 @@ defmodule Transport.IRVE.Extractor do
       Transport.IRVE.Fetcher.get!(row[:url], compressed: false, decode_body: false)
 
     row
-    |> Map.put(:status, status)
+    |> Map.put(:http_status, status)
     |> Map.put(:index, index)
     |> then(fn x -> process_resource_body(x, body) end)
   end
@@ -145,7 +146,7 @@ defmodule Transport.IRVE.Extractor do
   For a given resource and corresponding body, enrich data with
   extra stuff like estimated number of charge points.
   """
-  def process_resource_body(%{status: 200} = row, body) do
+  def process_resource_body(%{http_status: 200} = row, body) do
     body = body |> String.split("\n")
     first_line = body |> hd()
     line_count = (body |> length) - 1
@@ -160,7 +161,7 @@ defmodule Transport.IRVE.Extractor do
     |> Map.put(:line_count, line_count)
   end
 
-  def process_body(row), do: row
+  def process_resource_body(row, _body), do: row
 
   @doc """
   Save the outcome in the database for reporting.

apps/transport/lib/jobs/gtfs_to_db.ex

@@ -120,8 +120,8 @@ defmodule Transport.Jobs.GtfsToDB do
           friday: friday = r |> Map.fetch!("friday") |> String.to_integer(),
           saturday: saturday = r |> Map.fetch!("saturday") |> String.to_integer(),
           sunday: sunday = r |> Map.fetch!("sunday") |> String.to_integer(),
-          start_date: r |> Map.fetch!("start_date") |> Timex.parse!("{YYYY}{0M}{0D}") |> NaiveDateTime.to_date(),
-          end_date: r |> Map.fetch!("end_date") |> Timex.parse!("{YYYY}{0M}{0D}") |> NaiveDateTime.to_date()
+          start_date: r |> Map.fetch!("start_date") |> TimeWrapper.parse!("{YYYY}{0M}{0D}") |> NaiveDateTime.to_date(),
+          end_date: r |> Map.fetch!("end_date") |> TimeWrapper.parse!("{YYYY}{0M}{0D}") |> NaiveDateTime.to_date()
         }
 
         res
@@ -214,7 +214,7 @@ defmodule Transport.Jobs.GtfsToDB do
           %{
             data_import_id: data_import_id,
             service_id: r |> Map.fetch!("service_id"),
-            date: r |> Map.fetch!("date") |> Timex.parse!("{YYYY}{0M}{0D}") |> NaiveDateTime.to_date(),
+            date: r |> Map.fetch!("date") |> TimeWrapper.parse!("{YYYY}{0M}{0D}") |> NaiveDateTime.to_date(),
             exception_type: r |> Map.fetch!("exception_type") |> String.to_integer()
           }
         end)

apps/transport/lib/jobs/oban_logger.ex

@@ -1,8 +1,14 @@
 defmodule Transport.Jobs.ObanLogger do
   @moduledoc """
-  Logs the Oban job exceptions as warnings
+  Setup telemetry/logging for Oban.
+
+  We:
+  - log job exceptions as warnings
+  - log Oban events related to the orchestration (notifier, queues, plugins etc.)
+  - we send an email when a job failed after its maximum attempt for jobs with a specific tag
   """
   require Logger
+
   @tag_email_on_failure "email_on_failure"
 
   @doc """
@@ -35,5 +41,22 @@ defmodule Transport.Jobs.ObanLogger do
     )
   end
 
-  def setup, do: :telemetry.attach("oban-logger", [:oban, :job, :exception], &handle_event/4, nil)
+  def setup do
+    :telemetry.attach("oban-logger", [:oban, :job, :exception], &handle_event/4, nil)
+
+    # Log recommended events for production.
+    # We leave out `job` events because job start/end can be quite noisy.
+    # https://hexdocs.pm/oban/preparing_for_production.html#logging
+    # https://hexdocs.pm/oban/Oban.Telemetry.html
+    # We may simplify this when
+    # https://github.com/oban-bg/oban/commit/13eabe3f8019e350ef979369a26f186bdf7be63e
+    # will be released.
+    events = [
+      [:oban, :notifier, :switch],
+      [:oban, :queue, :shutdown],
+      [:oban, :stager, :switch]
+    ]
+
+    :telemetry.attach_many("oban-default-logger", events, &Oban.Telemetry.handle_event/4, encode: true, level: :info)
+  end
 end

apps/transport/lib/jobs/workflow.ex

@@ -82,7 +82,7 @@ defmodule Transport.Jobs.Workflow do
 
           {:notification, :gossip, %{"success" => false, "job_id" => ^job_id} = notif} ->
             reason = notif |> Map.get("reason", "unknown reason")
-            {:error, "Job #{job_id} has failed: #{inspect(reason)}. Workflow is stopping here"}
+            {:error, "Job #{job_id} has failed: #{reason}. Workflow is stopping here"}
         end
     end
   end
@@ -173,10 +173,17 @@ defmodule Transport.Jobs.Workflow do
         },
         nil
       ) do
+    # `error` can be an error message or an `Oban.TimeoutError` exception.
+    # ````
+    # %Oban.TimeoutError{
+    #   message: "Transport.Jobs.ResourceHistoryJob timed out after 1000ms",
+    #   reason: :timeout
+    # }
+    # ```
     Notifier.notify_workflow(%{meta: %{"workflow" => true}}, %{
       "success" => false,
       "job_id" => job_id,
-      "reason" => error
+      "reason" => inspect(error)
     })
   end
 

apps/transport/lib/transport_web/live/backoffice/irve_dashboard_live.ex

@@ -3,7 +3,7 @@ defmodule TransportWeb.Backoffice.IRVEDashboardLive do
   use Phoenix.HTML
   import TransportWeb.Backoffice.JobsLive, only: [ensure_admin_auth_or_redirect: 3]
   import TransportWeb.Router.Helpers
-  import Helpers, only: [format_number: 1]
+  import Helpers, only: [format_number_maybe_nil: 2]
   import Ecto.Query, only: [last: 2]
 
   @impl true
@@ -25,7 +25,12 @@ defmodule TransportWeb.Backoffice.IRVEDashboardLive do
 
     filtering_expression == "" ||
       String.contains?(resource["dataset_organisation_name"] |> String.downcase(), filtering_expression) ||
-      String.contains?(format_validity(resource["valid"]) |> inspect |> String.downcase(), filtering_expression)
+      String.contains?(
+        format_validity(resource["valid"], resource["http_status"])
+        |> inspect
+        |> String.downcase(),
+        filtering_expression
+      )
   end
 
   def assign_data(socket) do
@@ -119,7 +124,9 @@ defmodule TransportWeb.Backoffice.IRVEDashboardLive do
     |> Enum.sort_by(fn x -> x["line_count"] end, :desc)
   end
 
-  def format_validity(false), do: {:safe, "<strong class='red'>KO</strong>"}
-  def format_validity(true), do: "OK"
-  def format_validity(nil), do: "Non testé"
+  def format_validity(false, 200), do: {:safe, "<strong class='red'>KO</strong>"}
+  def format_validity(true, 200), do: "OK"
+  def format_validity(nil, 200), do: "Non testé"
+  # mostly there to handle 404/500. Ignore validity and assume the resource is not reachable at all
+  def format_validity(_validity, http_status), do: {:safe, "<strong class='red'>KO (#{http_status})</strong>"}
 end

apps/transport/lib/transport_web/live/backoffice/irve_dashboard_live.html.heex

@@ -43,9 +43,9 @@
               &mdash; <%= resource["last_modified"] |> String.slice(0..15) %>
             </td>
             <td>
-              <%= format_validity(resource["valid"]) %>
+              <%= format_validity(resource["valid"], resource["http_status"]) %>
             </td>
-            <td><%= format_number(resource["line_count"]) %></td>
+            <td><%= format_number_maybe_nil(resource["line_count"], nil_result: "???") %></td>
           </tr>
         <% end %>
       </tbody>

apps/transport/lib/transport_web/live/discussions_live.ex

@@ -115,7 +115,7 @@ defmodule TransportWeb.DiscussionsLive do
       end)
       |> Enum.max(DateTime)
 
-    two_months_ago = DateTime.utc_now() |> Timex.shift(months: -2)
+    two_months_ago = DateTime.utc_now() |> TimeWrapper.shift(months: -2)
     DateTime.compare(two_months_ago, latest_comment_datetime) == :gt
   end
 

apps/transport/lib/transport_web/plugs/router.ex

@@ -2,7 +2,7 @@ defmodule TransportWeb.Plugs.Router do
   use Plug.Router
 
   plug(TransportWeb.Plugs.HealthCheck, at: "/health-check")
-  plug(TransportWeb.Plugs.Halt, if: {Transport.Application, :worker_only?}, message: "UP (WORKER-ONLY)")
+  plug(TransportWeb.Plugs.WorkerHealthcheck, if: {Transport.Application, :worker_only?})
 
   plug(:match)
   plug(:dispatch)