Support device deletion #816

Annopaolo · 2023-07-06T12:48:14Z

A device may be deleted with a DELETE request on the /:realm_name/devices/:device_id endpoint of Realm Management API. Deletion is then performed asynchronously.
When a device is being deleted, it is also disconnected from Astarte. Already stored data may still be available until deletion is complete. All inflight data coming from the device are discarded. A synchronization mechanism makes sure that data deletion is performed only when all inflight messages have been handled (i.e. discarded) and the device is not connected to Astarte.

In detail, the deletion procedure follows the steps outlined below:

RM API forwards the request via RPC to RM (see also Add support for device deletion astarte_rpc#63)
RM inserts the device_id in the deletion_in_progress table
RM answers to the RPC and RM API answers with 204 ok to the client
Each DataUpdater process checks periodically if its device is being deleted. If that's the case:
- The DataUpdater process updates its deletion_in_progress flag to true and writes the dup_start_ack in the deletion_in_progress table. From now on, all received messages will be acked but ignored.
- The DataUpdater performs an RPC call to the broker (see also Support device deletion astarte_vmq_plugin#75 and Add support for device deletion astarte_rpc#63) to disconnect the device
In order to delete offline (or old) devices, too, the DUP RemovalScheduler periodically checks the deletion_in_progress table and starts the DataUpdater process of offline (or old) devices with a :start_device_deletion message.
When the broker receives the RPC from DUP, it forcefully disconnects the device and will not allow any reconnection until that device_id is in the deletion_in_progress table. Then the broker writes the vmq_ack in the deletion_in_progress table and sends an empty message on the "/internal" topic related to the device to signal that messages from the device are finished.
When the DataUpdater process handles the empty termination message, it writes the dup_end_ack in the deletion_in_progress table and terminates itself
RM periodically checks the deletion_in_progress table and spawns a task for deleting data from devices whose deletion has been acked both by the broker and the DataUpdater.

Related to astarte-platform/astarte_vmq_plugin#75 and based, based on astarte-platform/astarte_rpc#63.
Closes #392.

Annopaolo · 2023-07-06T15:25:45Z

Do not merge until astarte-platform/astarte_vmq_plugin#75 and astarte-platform/astarte_rpc#63 are merged.

codecov · 2023-07-07T11:28:02Z

Codecov Report

Merging #816 (7c30db5) into master (e0fd426) will increase coverage by 0.49%.
Report is 14 commits behind head on master.
The diff coverage is 71.23%.

❗ Current head 7c30db5 differs from pull request most recent head a47a2a8. Consider uploading reports for the commit a47a2a8 to get more accurate results

@@            Coverage Diff             @@
##           master     #816      +/-   ##
==========================================
+ Coverage   67.42%   67.92%   +0.49%     
==========================================
  Files         264      269       +5     
  Lines        6429     6781     +352     
==========================================
+ Hits         4335     4606     +271     
- Misses       2094     2175      +81

Files	Coverage Δ
...astarte_data_updater_plant/consumers_supervisor.ex	`100.00% <ø> (ø)`
...ant/lib/astarte_data_updater_plant/data_updater.ex	`94.20% <100.00%> (+0.26%)`	⬆️
.../astarte_data_updater_plant/data_updater/server.ex	`87.27% <100.00%> (+6.88%)`	⬆️
...t/lib/astarte_data_updater_plant/rpc/vmq_plugin.ex	`29.41% <100.00%> (+29.41%)`	⬆️
...e_realm_management/lib/astarte_realm_management.ex	`100.00% <100.00%> (ø)`
...ib/astarte_realm_management_api/devices/devices.ex	`100.00% <100.00%> (ø)`
...tarte_realm_management_api/rpc/realm_management.ex	`56.81% <100.00%> (ø)`
...anagement_api_web/controllers/device_controller.ex	`100.00% <100.00%> (ø)`
...api/lib/astarte_realm_management_api_web/router.ex	`77.77% <100.00%> (+2.77%)`	⬆️
..._api/test/support/astarte_realm_management_mock.ex	`94.11% <100.00%> (+0.36%)`	⬆️
... and 14 more

... and 6 files with indirect coverage changes

noaccOS

I have reported a couple instances where the copyright signature was incorrect, mainly in new files, but in most updated files the copyright wasn't updated.

There is a lot of formatting changes, mainly in 03d5cb2 and 445711b, which make a lot of unnecessary noise in the diff. Consider at least adding a separate commit for the formatting before the commit with actual changes.

I don't know how much/if it is a problem but 445711b isn't formatted with mix format

noaccOS · 2023-08-18T09:46:36Z

apps/astarte_data_updater_plant/lib/astarte_data_updater_plant/data_updater/queries.ex

        Logger.warn("Database error while retrieving property: #{inspect(reason)}.")
+
+        {:error, :database_error}


I think this line was a mistake as the previous entry in the else block doesn't have an empty line between the Logger.warn and the returned tuple

noaccOS · 2023-08-18T09:49:14Z

apps/astarte_data_updater_plant/lib/astarte_data_updater_plant/data_updater/queries.ex

+    with {:ok, _} <- DatabaseQuery.call(client, query) do
+      :ok
+    else
+      %{acc: _, msg: error_message} ->
+        Logger.warn("Database error when writing end ack device deletion: #{error_message}.")
+
+        {:error, :database_error}
+
+      {:error, reason} ->
+        Logger.warn("Device deletion end ack failed with reason: #{inspect(reason)}.")
+
+        {:error, :database_error}
+    end


Suggested change

with {:ok, _} <- DatabaseQuery.call(client, query) do

:ok

else

%{acc: _, msg: error_message} ->

Logger.warn("Database error when writing end ack device deletion: #{error_message}.")

{:error, :database_error}

{:error, reason} ->

Logger.warn("Device deletion end ack failed with reason: #{inspect(reason)}.")

{:error, :database_error}

end

case DatabaseQuery.call(client, query) do

{:ok, _} ->

:ok

%{acc: _, msg: error_message} ->

Logger.warn("Database error when writing end ack device deletion: #{error_message}.")

{:error, :database_error}

{:error, reason} ->

Logger.warn("Device deletion end ack failed with reason: #{inspect(reason)}.")

{:error, :database_error}

end

is there a reason we're not using Xandra for new queries?
retrieve_realms!/0 and retrieve_devices_waiting_to_start_deletion!/1 in this file are already using it.
I get that ack_start_device_deletion/2 and check_device_deletion_in_progress/2 are used in a context with a db_client already defined, but this is not the case for ack_end_device_deletion/2

++, let's use Xandra for new queries

noaccOS · 2023-08-18T09:49:30Z

apps/astarte_data_updater_plant/lib/astarte_data_updater_plant/data_updater/queries.ex

+    with {:ok, _} <- DatabaseQuery.call(client, query) do
+      :ok
+    else
+      %{acc: _, msg: error_message} ->
+        Logger.warn("Database error when writing start ack device deletion: #{error_message}.")
+
+        {:error, :database_error}
+
+      {:error, reason} ->
+        Logger.warn("Device deletion start ack failed with reason: #{inspect(reason)}.")
+
+        {:error, :database_error}
+    end


same as before

noaccOS · 2023-08-18T09:54:08Z

apps/astarte_data_updater_plant/lib/astarte_data_updater_plant/data_updater/queries.ex

+    with {:ok, result} <- DatabaseQuery.call(client, query),
+         deletion_row when is_list(deletion_row) <- DatabaseResult.head(result) do
+      {:ok, true}
+    else
+      :empty_dataset ->
+        {:ok, false}
+
+      %{acc: _, msg: error_message} ->
+        _ = Logger.warn("Database error: #{error_message}.", tag: "db_error")
+        {:error, :database_error}
+
+      {:error, reason} ->
+        _ =
+          Logger.warn("Database error, reason: #{inspect(reason)}.",
+            tag: "db_error"
+          )
+
        {:error, :database_error}
    end


Suggested change

with {:ok, result} <- DatabaseQuery.call(client, query),

deletion_row when is_list(deletion_row) <- DatabaseResult.head(result) do

{:ok, true}

else

:empty_dataset ->

{:ok, false}

%{acc: _, msg: error_message} ->

_ = Logger.warn("Database error: #{error_message}.", tag: "db_error")

{:error, :database_error}

{:error, reason} ->

_ =

Logger.warn("Database error, reason: #{inspect(reason)}.",

tag: "db_error"

)

{:error, :database_error}

end

case DatabaseQuery.call(client, query) do

{:ok, result} ->

result_not_empty? = DatabaseResult.head(result) != :empty_dataset

{:ok, result_not_empty?}

%{acc: _, msg: error_message} ->

_ = Logger.warn("Database error: #{error_message}.", tag: "db_error")

{:error, :database_error}

{:error, reason} ->

_ =

Logger.warn("Database error, reason: #{inspect(reason)}.",

tag: "db_error"

)

{:error, :database_error}

end

noaccOS · 2023-08-18T09:54:39Z

apps/astarte_data_updater_plant/lib/astarte_data_updater_plant/data_updater/queries.ex

+    with {:ok, result} <- DatabaseQuery.call(client, query),
+         deletion_row when is_list(deletion_row) <- DatabaseResult.head(result) do
+      {:ok, true}
+    else
+      :empty_dataset ->
+        {:ok, false}
+
+      %{acc: _, msg: error_message} ->
+        _ = Logger.warn("Database error: #{error_message}.", tag: "db_error")
+        {:error, :database_error}
+
+      {:error, reason} ->
+        _ =
+          Logger.warn("Database error, reason: #{inspect(reason)}.",
+            tag: "db_error"
+          )
+
+        {:error, :database_error}
+    end


same as before

noaccOS · 2023-08-18T13:14:48Z

apps/astarte_realm_management/lib/astarte_realm_management/device_removal/device_remover.ex

+#
+# This file is part of Astarte.
+#
+# Copyright 20230 SECO Mind Srl


Suggested change

# Copyright 20230 SECO Mind Srl

# Copyright 2023 SECO Mind Srl

noaccOS · 2023-08-21T07:22:10Z

apps/astarte_realm_management/test/astarte_realm_management/device_remover_test.exs

+  setup_all do
+    with {:ok, client} <- DatabaseTestHelper.connect_to_test_database() do
+      DatabaseTestHelper.create_test_keyspace(client)
+      seed_device_data!()
+    end
+
+    on_exit(fn ->
+      with {:ok, client} <- DatabaseTestHelper.connect_to_test_database() do
+        DatabaseTestHelper.drop_test_keyspace(client)
+      end
+    end)
+  end


It doesn't really matter as we only have one test for now, but being assertive and failing in the setup makes the error message more readable as we only have one global database message for the file and not one for each test

Suggested change

setup_all do

with {:ok, client} <- DatabaseTestHelper.connect_to_test_database() do

DatabaseTestHelper.create_test_keyspace(client)

seed_device_data!()

end

on_exit(fn ->

with {:ok, client} <- DatabaseTestHelper.connect_to_test_database() do

DatabaseTestHelper.drop_test_keyspace(client)

end

end)

end

setup_all do

{:ok, client} = DatabaseTestHelper.connect_to_test_database()

DatabaseTestHelper.create_test_keyspace(client)

seed_device_data!()

on_exit(fn ->

{:ok, client} = DatabaseTestHelper.connect_to_test_database()

DatabaseTestHelper.drop_test_keyspace(client)

end)

end

noaccOS · 2023-08-21T07:36:07Z

apps/astarte_realm_management/test/astarte_realm_management/queries_test.exs

+    # my sanity is slowly degrading
+    aliaz = "ahahahah now I can write 'alias' without Elixir complaining"


how about device_alias?

noaccOS · 2023-08-21T08:05:33Z

..._management_api/test/astarte_realm_management_api_web/controllers/device_controller_test.exs

+#
+# This file is part of Astarte.
+#
+# Copyright 2018 Ispirata Srl


Suggested change

# Copyright 2018 Ispirata Srl

# Copyright 2023 SECO Mind Srl

noaccOS · 2023-08-21T12:40:04Z

apps/astarte_realm_management/lib/device_remover.ex

Is the plan to squash and merge this pr? If not, there are a few files in b663702 and 2c0bb89 which have been renamed mid-pr, and I think it would be better to avoid it

rbino · 2023-08-25T14:03:48Z

...astarte_data_updater_plant/lib/astarte_data_updater_plant/data_updater/deletion_scheduler.ex

+
+  # TODO expose this via config
+  # 5 minutes
+  @reconciliation_timeout 300 * 1000


:timer.minutes(5)

rbino · 2023-08-25T14:27:36Z

apps/astarte_data_updater_plant/lib/astarte_data_updater_plant/data_updater/queries.ex

+    with {:ok, _} <- DatabaseQuery.call(client, query) do
+      :ok
+    else
+      %{acc: _, msg: error_message} ->
+        Logger.warn("Database error when writing end ack device deletion: #{error_message}.")
+
+        {:error, :database_error}
+
+      {:error, reason} ->
+        Logger.warn("Device deletion end ack failed with reason: #{inspect(reason)}.")
+
+        {:error, :database_error}
+    end


++, let's use Xandra for new queries

astarte_core, astarte_rpc, astarte_data_access Signed-off-by: Arnaldo Cesco <[email protected]>

Annopaolo · 2023-08-29T15:16:55Z

I have reported a couple instances where the copyright signature was incorrect, mainly in new files, but in most updated files the copyright wasn't updated.

There is a lot of formatting changes, mainly in 03d5cb2 and 445711b, which make a lot of unnecessary noise in the diff. Consider at least adding a separate commit for the formatting before the commit with actual changes.

I don't know how much/if it is a problem but 445711b isn't formatted with mix format

Other than the inline suggestions, these have been tackled, too (updated copyright, removed unnecessary formatting changes). TY @noaccOS!

noaccOS

LGTM

rbino · 2023-08-31T16:19:12Z

...astarte_data_updater_plant/lib/astarte_data_updater_plant/data_updater/deletion_scheduler.ex

+  end
+
+  def init(_args) do
+    {:ok, %{}, {:continue, []}}


Given handle_continue just sends a single message, handling this with handle_continue adds an unnecessary indirection

rbino · 2023-08-31T16:23:46Z

...astarte_data_updater_plant/lib/astarte_data_updater_plant/data_updater/deletion_scheduler.ex

+    {:noreply, state}
+  end
+
+  defp start_device_deletion!() do


No need for parentheses for declaration with no arguments. Also, imho having a start_device_deletion! and a start_device_deletion which actually do two different things is actually quite confusing. Given start_device_deletion is 3 lines and it's only used once, I think you can just move its implementation in the anonymous function called by Enum.each

rbino · 2023-09-01T10:58:24Z

...astarte_data_updater_plant/lib/astarte_data_updater_plant/data_updater/deletion_scheduler.ex

+    Enum.flat_map(realms, fn %{"realm_name" => realm_name} ->
+      devices = Queries.retrieve_devices_waiting_to_start_deletion!(realm_name)
+      Enum.map(devices, &Map.put(&1, "realm_name", realm_name))
+    end)
+    |> Enum.filter(fn %{"realm_name" => realm_name, "device_id" => device_id} ->
+      should_handle_data_from_device?(realm_name, device_id)
+    end)


Using the ability of for-comprehension of doing nested loop and filtering this could be made more readable

for %{"realm_name" => realm_name} <- realms, %{"device_id" => device_id} <- Queries.retrieve_devices_waiting_to_start_deletion!(realm_name), encoded_device_id = Device.encode_device_id(device_id), should_handle_data_from_device?(realm_name, encoded_device_id) do %{realm_name: realm_name, device_id: encoded_device_id} end

Moreover I switched the returned map to an atom-keyed map with only the fields you need. This makes the intent more clear by asserting that you're only using those two keys from the result, while if you take the result from the DB and do Map.put the reader is not sure if you're using the other fields or not (which you are not).
Also, I encode the device ID here so it doesn't have to be encoded twice in should_handle... and start_device_deletion. Feel free to change the key to encoded_device_id: if you think it's clearer.

rbino · 2023-09-01T10:59:59Z

...astarte_data_updater_plant/lib/astarte_data_updater_plant/data_updater/deletion_scheduler.ex

+    |> tap(fn devices ->
+      Logger.debug("Retrieved devices to delete #{inspect(devices)}", tag: "devices_to_delete")
+    end)


I'd move this inside the loop and log device-per-device, so you can add realm and device_id as log metadata, making it more discoverable with log parsing (while just dumping the list of maps is not parsable by Loki and co)

rbino · 2023-09-04T10:46:53Z

apps/astarte_data_updater_plant/lib/astarte_data_updater_plant/data_updater/impl.ex

@@ -98,6 +101,11 @@ defmodule Astarte.DataUpdaterPlant.DataUpdater.Impl do
    :ok
  end

+  def handle_connection(%State{drop_messages: true} = state, _, message_id, _) do
+    MessageTracker.ack_delivery(state.message_tracker, message_id)


Given we're throwing these away, I'd say that using discard is more semantically accurate (here and in all similar clauses)

@Annopaolo I think this wasn't fixed

rbino · 2023-09-04T15:11:25Z

apps/astarte_data_updater_plant/lib/astarte_data_updater_plant/data_updater/queries.ex

+    Xandra.Cluster.run(
+      :xandra,
+      &do_ack_end_device_deletion(&1, realm_name, device_id)
+    )


You should validate the realm name like we do in Astarte Data Access, otherwise you're interpolating a string in a query without checking it

This will be handled by #838 once rebased on this PR.

rbino · 2023-09-04T15:17:40Z

apps/astarte_housekeeping/priv/migrations/realm/0005_create_deletion_in_progress_table.sql

+  vmq_ack boolean,
+  dup_start_ack boolean,
+  dup_end_ack boolean,
+  PRIMARY KEY ((device_id))


No need for the double parentheses

rbino · 2023-09-04T15:27:08Z

apps/astarte_realm_management/lib/astarte_realm_management/queries.ex

+  defp do_check_device_exists(conn, realm_name, device_id) do
+    statement = """
+    SELECT COUNT (*)
+    FROM #{realm_name}.devices


Same here, realm_name should be checked.

rbino · 2023-09-04T15:53:16Z

apps/astarte_realm_management/lib/device_remover.ex

+  defp filter_object_interfaces(realm_name, introspection) do
+    Enum.reduce_while(introspection, {:ok, []}, fn {interface_name, interface_major}, acc ->
+      case Queries.retrieve_interface_descriptor(realm_name, interface_name, interface_major) do
+        # TODO check


rbino · 2023-09-04T16:09:25Z

apps/astarte_realm_management/test/support/database_test_helper.exs

+      statement = """
+      INSERT INTO #{realm_name}.individual_datastreams
+        (device_id, interface_id, endpoint_id, path, value_timestamp, reception_timestamp, reception_timestamp_submillis, integer_value)
+      VALUES (:device_id, :interface_id, :endpoint_id, :path, '2017-09-28 04:06+0000', '2017-09-28 05:06+0000', 0, 42);


Pass also the values you insert here as a parameter instead of hardcoding them, otherwise the known value magically appears in assertions in the test case and it's not clear where it is coming from.

rbino · 2023-09-04T16:14:20Z

apps/astarte_realm_management_api/test/astarte_realm_management_api/devices/devices_test.exs

+    assert :ok = Devices.delete_device(@realm, device_id)
+  end
+
+  test "install fails when the device does not exists" do


rbino · 2023-09-04T16:16:26Z

apps/astarte_realm_management/lib/device_remover.ex

+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+defmodule Astarte.RealmManagement.DeviceRemover do


Actually, I think this whole file is a leftover

rbino · 2023-09-04T16:20:07Z

apps/astarte_realm_management/lib/astarte_realm_management/device_removal/device_remover.ex

+    _ = Logger.info("Starting to remove device #{encoded_device_id}", tag: "device_delete_start")
+
+    datastream_keys = Queries.retrieve_individual_datastreams_keys!(realm_name, device_id)
+    :ok = delete_individual_datastreams!(realm_name, datastream_keys)


Matching with :ok on bang functions is redundant and adds noise, I'd drop it

rbino · 2023-09-04T16:37:55Z

apps/astarte_realm_management/lib/astarte_realm_management/device_removal/device_remover.ex

+    introspection = retrieve_device_introspection_map!(realm_name, device_id)
+    object_interfaces = filter_object_interfaces!(realm_name, introspection)
+    object_tables = Enum.map(object_interfaces, &object_interface_to_table_name/1)
+


Sometimes directly calling Enum functions and some other times defining private helpers to call them, I'd prefer a consistent behavior. I'd suggest moving the Enum.each here and leaving as helper only the function that unpacks the struct and uses that to create the arguments for the function call.
If you combine this with dropping the match on :ok, you can easily group functions in pipes which end up handling a specific side effect, e.g:

# Delete individual datastreams Queries.retrieve_individual_datastreams_keys!(realm_name, device_id) |> Enum.each(&delete_individual_datastreams_from_key!/1) ... end) defp delete_individual_datastreams_from_key!(key) do %{ device_id: device_id, interface_id: interface_id, endpoint_id: endpoint_id, path: path } = key Queries.delete_individual_datastream_values!( realm_name, device_id, interface_id, endpoint_id, path ) end

Signed-off-by: Arnaldo Cesco <[email protected]>

Device deletion begins when a device_id is put in the `deletion_in_progress` table. The actual deletion of data is started only when it has been acknowledged by both DUP (two times) and the broker (once). The DeviceRemoverSupervisor supervises DeviceRemover tasks. Since deletion is asynchronous, it may happen that the RM service goes down during deletion, or that a temporary filure brings down the deletion service. Therefore, a supervision tree is put in place. Data deletion is started (or resumed, if needed) by the DeviceRemoval.Scheduler and it is carried out by the DeviceRemoval.DeviceRemover Task. The DeviceRemoverSupervisor supervises DeviceRemovers.

astarte_core and astarte_rpc Signed-off-by: Arnaldo Cesco <[email protected]>

Allow to delete a device with an HTTP request using the DELETE method on the `/{realm_name}/devices/{device_id}` endpoint. Signed-off-by: Arnaldo Cesco <[email protected]>

Signed-off-by: Arnaldo Cesco <[email protected]>

astarte_core, astarte_rpc, astarte-data_access Signed-off-by: Arnaldo Cesco <[email protected]>

Add a migration for creating the `deletion_in_progress` table. Signed-off-by: Arnaldo Cesco <[email protected]>

Track devices currently under deletion in the `deletion_in_progress` table. It also contains fields to track the acknowledgement of the deletion by DUP and the broker. Signed-off-by: Arnaldo Cesco <[email protected]>

Bump cyanide, astarte_core, astarte_rpc, astarte-data_access. Contextually, remove some outdated macros. Signed-off-by: Arnaldo Cesco <[email protected]>

A DataUpdater process checks periodically if the related device is being deleted. If that is the case, the process updates its `deletion_in_progress` state subfield to `true` and writes to db that deletion has started (`dup_start_ack` field). From now on, all received messages will be acked but ignored, in order to end consuming all inflight messages. The DataUpdater also performs an RPC to the broker to forcefully disconnect the device and prevent it from reconnecting. The last message to be processed by the DataUpdater is by construction received on the internal `"/f"` path (see astarte-platform/astarte_vmq_plugin#75). After that, the DUP deletion end ack is written to the database and the DataUpdater process is terminated. In order to allow deletion of offline (or old) devices, the RemovalScheduler periodically checks the `deletion_in_progress` table and makes the device process start just to perform the deletion check. Signed-off-by: Arnaldo Cesco <[email protected]>

Add the Mox library in order to mock the Astarte RPC client. Signed-off-by: Arnaldo Cesco <[email protected]>

Signed-off-by: Arnaldo Cesco <[email protected]>

Now the Astarte VerneMQ Plugin needs access to the astarte database. Add the relevant env var in the docker-compose.yml file to allow it to connect. Signed-off-by: Arnaldo Cesco <[email protected]>

Annopaolo added this to the v1.2 milestone Jul 6, 2023

Annopaolo requested review from bettio and rbino July 6, 2023 12:48

Annopaolo force-pushed the delete-device branch from c127cad to 3cb6bcb Compare July 6, 2023 15:23

Annopaolo force-pushed the delete-device branch from 3cb6bcb to fdb9459 Compare July 7, 2023 11:17

Annopaolo force-pushed the delete-device branch from fdb9459 to 9de6cd2 Compare July 12, 2023 08:57

Annopaolo force-pushed the delete-device branch 3 times, most recently from 1ea2a42 to f914059 Compare August 1, 2023 08:20

Annopaolo marked this pull request as ready for review August 4, 2023 12:35

noaccOS reviewed Aug 21, 2023

View reviewed changes

rbino requested changes Aug 25, 2023

View reviewed changes

RM: Bump astarte deps

e91a45d

astarte_core, astarte_rpc, astarte_data_access Signed-off-by: Arnaldo Cesco <[email protected]>

Annopaolo force-pushed the delete-device branch 5 times, most recently from e9be707 to 2d2e97e Compare August 29, 2023 14:53

Annopaolo requested review from rbino and noaccOS August 29, 2023 15:12

noaccOS approved these changes Aug 30, 2023

View reviewed changes

rbino requested changes Sep 1, 2023

View reviewed changes

Annopaolo force-pushed the delete-device branch from 2d2e97e to 8301a12 Compare September 4, 2023 14:33

rbino requested changes Sep 4, 2023

View reviewed changes

Annopaolo force-pushed the delete-device branch 3 times, most recently from 7defc29 to a3d3018 Compare September 8, 2023 10:36

Annopaolo requested a review from rbino September 8, 2023 11:19

Annopaolo added 12 commits September 8, 2023 16:47

RM: add device deletion queries

b82562a

Signed-off-by: Arnaldo Cesco <[email protected]>

RM: test device removal queries

595eb2d

Signed-off-by: Arnaldo Cesco <[email protected]>

RM: Test device deletion logic

a5486ef

RM: handle device deletion RPC

e52d94f

RM API: bump relevant deps

e8ca36f

astarte_core and astarte_rpc Signed-off-by: Arnaldo Cesco <[email protected]>

RM API: support device deletion

f1dab59

Allow to delete a device with an HTTP request using the DELETE method on the `/{realm_name}/devices/{device_id}` endpoint. Signed-off-by: Arnaldo Cesco <[email protected]>

RM API: test support for device deletion

40b7a44

Signed-off-by: Arnaldo Cesco <[email protected]>

HK: bump astarte core deps

6fc4348

astarte_core, astarte_rpc, astarte-data_access Signed-off-by: Arnaldo Cesco <[email protected]>

HK: support device deletion for pre-existing realms

0664550

Add a migration for creating the `deletion_in_progress` table. Signed-off-by: Arnaldo Cesco <[email protected]>

HK: add table to keep track of devices being deleted

85ea009

Track devices currently under deletion in the `deletion_in_progress` table. It also contains fields to track the acknowledgement of the deletion by DUP and the broker. Signed-off-by: Arnaldo Cesco <[email protected]>

DUP: bump astarte core deps

354d9b8

Bump cyanide, astarte_core, astarte_rpc, astarte-data_access. Contextually, remove some outdated macros. Signed-off-by: Arnaldo Cesco <[email protected]>

Annopaolo force-pushed the delete-device branch from a3d3018 to 23cde71 Compare September 8, 2023 14:47

Annopaolo added 4 commits October 6, 2023 10:47

DUP: enable mocks using Mox

3caf93a

Add the Mox library in order to mock the Astarte RPC client. Signed-off-by: Arnaldo Cesco <[email protected]>

DUP: add tests for supporting device deletion

4054ab8

Signed-off-by: Arnaldo Cesco <[email protected]>

Allow VerneMQ to work in docker compose setup

a47a2a8

Now the Astarte VerneMQ Plugin needs access to the astarte database. Add the relevant env var in the docker-compose.yml file to allow it to connect. Signed-off-by: Arnaldo Cesco <[email protected]>

Annopaolo force-pushed the delete-device branch from 23cde71 to a47a2a8 Compare October 6, 2023 08:48

rbino approved these changes Oct 6, 2023

View reviewed changes

rbino merged commit 01c8510 into astarte-platform:master Oct 6, 2023
29 checks passed

This was referenced Dec 12, 2023

Unable to delete device when no interfaces are present in the realm #885

Closed

Device deletion returns ok for non-existing devices #886

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support device deletion #816

Support device deletion #816

Annopaolo commented Jul 6, 2023 •

edited

Loading

Annopaolo commented Jul 6, 2023

codecov bot commented Jul 7, 2023 •

edited

Loading

noaccOS left a comment

noaccOS Aug 18, 2023

noaccOS Aug 18, 2023

rbino Aug 25, 2023

noaccOS Aug 18, 2023

noaccOS Aug 18, 2023

noaccOS Aug 18, 2023

noaccOS Aug 18, 2023

noaccOS Aug 21, 2023

noaccOS Aug 21, 2023

noaccOS Aug 21, 2023

noaccOS Aug 21, 2023

rbino Aug 25, 2023

rbino Aug 25, 2023

Annopaolo commented Aug 29, 2023

noaccOS left a comment

rbino Aug 31, 2023

rbino Aug 31, 2023

rbino Sep 1, 2023

rbino Sep 1, 2023

rbino Sep 4, 2023

rbino Oct 6, 2023

rbino Sep 4, 2023

Annopaolo Sep 5, 2023

rbino Sep 4, 2023

rbino Sep 4, 2023

rbino Sep 4, 2023

rbino Sep 4, 2023

rbino Sep 4, 2023

rbino Sep 4, 2023

rbino Sep 4, 2023

rbino Sep 4, 2023

		Logger.warn("Database error while retrieving property: #{inspect(reason)}.")

		{:error, :database_error}

	# Copyright 20230 SECO Mind Srl
	# Copyright 2023 SECO Mind Srl

		# my sanity is slowly degrading
		aliaz = "ahahahah now I can write 'alias' without Elixir complaining"

Support device deletion #816

Support device deletion #816

Conversation

Annopaolo commented Jul 6, 2023 • edited Loading

Annopaolo commented Jul 6, 2023

codecov bot commented Jul 7, 2023 • edited Loading

Codecov Report

noaccOS left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Annopaolo commented Aug 29, 2023

noaccOS left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Annopaolo commented Jul 6, 2023 •

edited

Loading

codecov bot commented Jul 7, 2023 •

edited

Loading