Add API actions to prune payloads collection #49

Yoann-Abbes · 2021-03-23T12:50:06Z

Add an API action to clean the payloads collection by days and/or valid payloads and/or deviceModel

Closes #27

Aschen · 2021-03-23T13:56:04Z

doc/1/controllers/device/clean/index.md

+### HTTP
+
+```http
+URL: http://kuzzle:7512/_/device-manager/devices/_clean_


Suggested change

URL: http://kuzzle:7512/_/device-manager/devices/_clean_

URL: http://kuzzle:7512/_/device-manager/devices/_clean

Aschen · 2021-03-23T13:56:28Z

doc/1/controllers/device/clean/index.md

+
+# clean
+
+Cleans the payload collection.


Can you add more info about the option available to clean the collection?

Aschen · 2021-03-23T13:57:36Z

doc/1/controllers/device/clean/index.md

+### Kourou
+
+```bash
+kourou device-manager/device:clean '{"days": <days>, "valid": "true|false", "deviceModel": "<deviceModel>"}'


Can you transform the object in a multiline object for more readability?
Also it's not JSON so you don't need " for properties name

Aschen · 2021-03-23T13:58:33Z

doc/1/controllers/device/clean/index.md

+## Arguments
+
+- `days`: Specify on which period of time you want keep payloads (`default: 7`).
+- `valid`: Specify which payloads are to be deleted (`default: true`).


Also this should not be a boolean so people can choose between valid, invalid or both

Suggested change

- `valid`: Specify which payloads are to be deleted (`default: true`).

- `valid`: Specify if valid, invalid or both payload should be deleted (`default: true`).

Aschen · 2021-03-23T13:58:56Z

doc/1/controllers/device/clean/index.md

+
+## Response
+
+Returns an array with the deleted payloads ids.


IMHO this is useless. Just the number of deleted payload is enough I think

Aschen · 2021-03-23T13:59:27Z

features/step_definitions/payloads-steps.js

@@ -27,3 +27,20 @@ Then(/I (successfully )?receive a "(.*?)" payload with:/, async function (expect
    this.props.error = error;
  }
 });
+
+Then(/I clean the payloads collection with args:/, async function (dataTable) {


Useless step

Aschen · 2021-03-23T14:00:12Z

lib/controllers/CRUDController.ts

@@ -50,7 +50,7 @@ export class CRUDController extends NativeController {
   *
   * @param request
   */
-  async delete (request: KuzzleRequest) {
+  delete (request: KuzzleRequest) {


This should stay async since handler function should always return a promise.

Here if anything goes wrong the method will throw an exception and not return a rejected promise

Why only delete ? The others are not async

Hmm they all should be async then

Aschen · 2021-03-23T14:01:27Z

lib/controllers/DeviceController.ts

+    const filter = []
+    filter.push({
+        range: {
+          "_kuzzle_info.createdAt": {


Nitpicking: you should use single quote

Aschen

The action name should be more explicite since it's in the device controller, something like cleanPayloads

Aschen · 2021-03-23T14:55:54Z

Changelog label is missing

scottinet · 2021-03-24T13:57:39Z

doc/1/controllers/device/clean/index.md

+description: Cleans the payload collection
+---
+
+# cleanPayloads


(nitpicking) usually the associated term for that kind of operation is "prune": prunePayloads

I thought prune is used when everything is erased?

verb (used with object), pruned, prun·ing. * to cut or lop off (twigs, branches, or roots). * to cut or lop superfluous or undesired twigs, branches, or roots from; trim. * to rid or clear of (anything superfluous or undesirable).

This seems adequate to me, since this action seems to allow the removal of superfluous payloads.

doc/1/controllers/device/clean/index.md

scottinet · 2021-03-24T14:01:11Z

doc/1/controllers/device/clean/index.md

+## Arguments
+
+- `days`: Specify on which period of time you want keep payloads (`default: 7`).
+- `valid`: Specify if valid, invalid or both payload should be deleted (`default: true`).


How can we keep both valid and invalid payloads with a boolean option? 🤔

Hmmm it's a mistake, we can't

Co-authored-by: Sébastien Cottinet <[email protected]>

doc/1/controllers/device/prune-payloads/index.md

features/DeviceController.feature

scottinet · 2021-03-26T09:05:40Z

doc/1/controllers/device/prune-payloads/index.md

+## Arguments
+
+- `days`: The maximum age of a payload, in days (`default: 7`).
+- `valid`: Specify if valid or invalid payloads should be deleted (`default: true`).


Hm. This is unclear to me: if set to true, does that mean that valid payloads are removed, and invalid ones are kept? If so, then that's a weird choice of a default value.

Otherwise, we could also understand that if set to true, then valid payloads are kept, and invalid ones are removed. But then, the documentation is wrong (or at least misleading): this option sets the kind of payloads to keep, not the ones to be removed.

In any case, that sentence suggests that this option allows to choose which kind of payload to keep (on or the other), but I don't think that this is how it's supposed to work.

Also, what is an "invalid payload"? This might be explained somewhere else in this documentation, but it seems weird that we keep those if they are invalid 🤔

I have a proposal to make things clearer, but we might need to modify it depending on what's the definition of an "invalid payload":

Suggested change

- `valid`: Specify if valid or invalid payloads should be deleted (`default: true`).

- `consolidate`: if set to true, ensure that only still-valid payloads are kept (`default: true`).

Invalid payload are the most interesting one because they are payload that couldn't be decoded properly by the associated decoder.
Developers can replay those payloads and try to understand how to decode them.
Maybe we can have an option keepInvalid so by default they are all deleted but we can choose to keep to invalid one for further analysis.

I agree with @Aschen on this one
Wdyt @scottinet ?

I'm ok with that.

Co-authored-by: Michele Leo <[email protected]>

Yoann-Abbes added 2 commits March 18, 2021 17:12

clean-payloads-api-actions

4e64c5f

doc and test

b6a8304

Yoann-Abbes self-assigned this Mar 23, 2021

Yoann-Abbes linked an issue Mar 23, 2021 that may be closed by this pull request

Add API action to clean the payloads collection #27

Closed

Aschen suggested changes Mar 23, 2021

View reviewed changes

@Aschen requested changes

f0d1164

Yoann-Abbes requested a review from Aschen March 23, 2021 14:17

nit

031e78f

Aschen suggested changes Mar 23, 2021

View reviewed changes

Change method name / asyncify CRUD methods

d570b77

Yoann-Abbes requested a review from Aschen March 23, 2021 14:52

Aschen approved these changes Mar 23, 2021

View reviewed changes

Yoann-Abbes added the changelog:new-features label Mar 23, 2021

scottinet suggested changes Mar 24, 2021

View reviewed changes

Yoann-Abbes and others added 2 commits March 24, 2021 17:28

Update doc/1/controllers/device/clean/index.md

8e394ff

Co-authored-by: Sébastien Cottinet <[email protected]>

@scottinet requested changes

b2266a2

Yoann-Abbes requested a review from scottinet March 24, 2021 16:37

Leodau approved these changes Mar 25, 2021

View reviewed changes

doc/1/controllers/device/prune-payloads/index.md Outdated Show resolved Hide resolved

features/DeviceController.feature Outdated Show resolved Hide resolved

scottinet suggested changes Mar 26, 2021

View reviewed changes

change option to keep invalid payloads

1daff29

Yoann-Abbes requested a review from scottinet March 31, 2021 08:40

Yoann-Abbes and others added 2 commits March 31, 2021 10:40

Update doc/1/controllers/device/prune-payloads/index.md

483a85c

Co-authored-by: Michele Leo <[email protected]>

Update features/DeviceController.feature

6e9b1f2

Co-authored-by: Michele Leo <[email protected]>

Aschen changed the title ~~Clean payloads collection API action~~ Add API actions to prune payloads collection Apr 2, 2021

Aschen merged commit 62ff5a8 into 1-dev Apr 2, 2021

Aschen deleted the clean-payloads-api-actions branch April 2, 2021 12:38

Yoann-Abbes mentioned this pull request May 4, 2021

Release 0.2.0 #74

Merged

alexandrebouthinon unassigned Yoann-Abbes Sep 24, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add API actions to prune payloads collection #49

Add API actions to prune payloads collection #49

Yoann-Abbes commented Mar 23, 2021

Aschen Mar 23, 2021

Aschen Mar 23, 2021

Aschen Mar 23, 2021

Aschen Mar 23, 2021

Aschen Mar 23, 2021

Aschen Mar 23, 2021

Aschen Mar 23, 2021

Yoann-Abbes Mar 23, 2021

Aschen Mar 23, 2021

Aschen Mar 23, 2021

Aschen left a comment

Aschen commented Mar 23, 2021

scottinet Mar 24, 2021

Yoann-Abbes Mar 24, 2021 •

edited

Loading

scottinet Mar 24, 2021 •

edited

Loading

scottinet Mar 24, 2021

Yoann-Abbes Mar 24, 2021

scottinet Mar 26, 2021 •

edited

Loading

Aschen Mar 26, 2021 •

edited

Loading

Yoann-Abbes Mar 26, 2021

scottinet Mar 30, 2021

	URL: http://kuzzle:7512/_/device-manager/devices/_clean_
	URL: http://kuzzle:7512/_/device-manager/devices/_clean

	- `valid`: Specify which payloads are to be deleted (`default: true`).
	- `valid`: Specify if valid, invalid or both payload should be deleted (`default: true`).

	- `valid`: Specify if valid or invalid payloads should be deleted (`default: true`).
	- `consolidate`: if set to true, ensure that only still-valid payloads are kept (`default: true`).

Add API actions to prune payloads collection #49

Add API actions to prune payloads collection #49

Conversation

Yoann-Abbes commented Mar 23, 2021

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

Aschen left a comment

Choose a reason for hiding this comment

Aschen commented Mar 23, 2021

Choose a reason for hiding this comment

Yoann-Abbes Mar 24, 2021 • edited Loading

Choose a reason for hiding this comment

scottinet Mar 24, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

scottinet Mar 26, 2021 • edited Loading

Choose a reason for hiding this comment

Aschen Mar 26, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yoann-Abbes Mar 24, 2021 •

edited

Loading

scottinet Mar 24, 2021 •

edited

Loading

scottinet Mar 26, 2021 •

edited

Loading

Aschen Mar 26, 2021 •

edited

Loading