Project

General

Profile

Actions
Copy link

Architecture #5345

closed

Non used parameters in an API call should lead to an error

Added by Nicolas PERRON over 9 years ago. Updated 8 months ago.

Status:
Rejected
Priority:
3
Assignee:
-
Category:
API
Target version:
-
Pull Request:
Effort required:
Large
Name check:
Fix check:
Regression:
No

Description

After trying to rename a parameter from "rudder_file_edit_footer" to "my_param" with API the result is:

$ curl -k -H "X-API-Token: PojCJDhajkGqVx1TTmiZKNFZORKOYc8X" -X POST https://localhost/rudder/api/2/parameters/rudder_file_edit_footer?prettify=true -d "id=my_param" 

{
  "action": "updateParameter",
  "id": "rudder_file_edit_footer",
  "result": "success",
  "data": {
    "parameters": [
      {
        "id": "rudder_file_edit_footer",
        "value": "### Edited by Rudder ###",
        "description": "Default inform message put in footer of managed files by Rudder",
        "overridable": false
      }
    ]
  }
}

The id has not changed.

If we try to do it manually by the WebUI, this is not possible since the id textbox is not editable. On the Eventlogs, policy generations are logged but not any modification.

I suppose this action with API should return an error, not a success.

Actions
Copy link
 #1

Updated by Nicolas PERRON over 9 years ago

  • Target version changed from 2.10.4 to 2.10.5
Actions
Copy link
 #2

Updated by Vincent MEMBRÉ over 9 years ago

  • Target version changed from 2.10.5 to 2.10.6
Actions
Copy link
 #3

Updated by Matthieu CERDA over 9 years ago

  • Target version changed from 2.10.6 to 2.10.7
Actions
Copy link
 #4

Updated by Vincent MEMBRÉ over 9 years ago

  • Target version changed from 2.10.7 to 2.10.8
Actions
Copy link
 #5

Updated by Vincent MEMBRÉ over 9 years ago

  • Target version changed from 2.10.8 to 2.10.9
Actions
Copy link
 #6

Updated by Vincent MEMBRÉ about 9 years ago

  • Target version changed from 2.10.9 to 2.10.10
Actions
Copy link
 #7

Updated by Vincent MEMBRÉ about 9 years ago

  • Target version changed from 2.10.10 to 2.10.11
Actions
Copy link
 #8

Updated by Vincent MEMBRÉ about 9 years ago

  • Target version changed from 2.10.11 to 2.10.12
Actions
Copy link
 #9

Updated by Vincent MEMBRÉ about 9 years ago

  • Target version changed from 2.10.12 to 2.10.13
Actions
Copy link
 #10

Updated by Vincent MEMBRÉ about 9 years ago

  • Target version changed from 2.10.13 to 2.10.14
Actions
Copy link
 #11

Updated by Vincent MEMBRÉ almost 9 years ago

  • Target version changed from 2.10.14 to 2.10.15
Actions
Copy link
 #12

Updated by Vincent MEMBRÉ almost 9 years ago

  • Target version changed from 2.10.15 to 2.10.16
Actions
Copy link
 #13

Updated by Vincent MEMBRÉ over 8 years ago

  • Target version changed from 2.10.16 to 2.10.17
Actions
Copy link
 #14

Updated by Vincent MEMBRÉ over 8 years ago

  • Target version changed from 2.10.17 to 2.10.18
Actions
Copy link
 #15

Updated by Vincent MEMBRÉ over 8 years ago

  • Target version changed from 2.10.18 to 2.10.19
Actions
Copy link
 #16

Updated by Vincent MEMBRÉ over 8 years ago

  • Target version changed from 2.10.19 to 2.10.20
Actions
Copy link
 #17

Updated by Vincent MEMBRÉ over 8 years ago

  • Target version changed from 2.10.20 to 277
Actions
Copy link
 #18

Updated by Vincent MEMBRÉ over 8 years ago

  • Target version changed from 277 to 2.11.18
Actions
Copy link
 #19

Updated by Vincent MEMBRÉ about 8 years ago

  • Target version changed from 2.11.18 to 2.11.19
Actions
Copy link
 #20

Updated by Vincent MEMBRÉ about 8 years ago

  • Target version changed from 2.11.19 to 2.11.20
Actions
Copy link
 #21

Updated by Vincent MEMBRÉ about 8 years ago

  • Target version changed from 2.11.20 to 2.11.21
Actions
Copy link
 #22

Updated by Vincent MEMBRÉ almost 8 years ago

  • Target version changed from 2.11.21 to 2.11.22
Actions
Copy link
 #23

Updated by Vincent MEMBRÉ almost 8 years ago

  • Target version changed from 2.11.22 to 2.11.23
Actions
Copy link
 #24

Updated by Vincent MEMBRÉ over 7 years ago

  • Target version changed from 2.11.23 to 2.11.24
Actions
Copy link
 #25

Updated by Vincent MEMBRÉ over 7 years ago

  • Target version changed from 2.11.24 to 308
Actions
Copy link
 #26

Updated by Vincent MEMBRÉ over 7 years ago

  • Target version changed from 308 to 3.1.14
Actions
Copy link
 #27

Updated by Vincent MEMBRÉ over 7 years ago

  • Target version changed from 3.1.14 to 3.1.15
Actions
Copy link
 #28

Updated by Vincent MEMBRÉ over 7 years ago

  • Target version changed from 3.1.15 to 3.1.16
Actions
Copy link
 #29

Updated by Vincent MEMBRÉ over 7 years ago

  • Target version changed from 3.1.16 to 3.1.17
Actions
Copy link
 #30

Updated by Vincent MEMBRÉ over 7 years ago

  • Target version changed from 3.1.17 to 3.1.18
Actions
Copy link
 #31

Updated by Vincent MEMBRÉ about 7 years ago

  • Target version changed from 3.1.18 to 3.1.19
Actions
Copy link
 #32

Updated by François ARMAND about 7 years ago

  • Severity set to Minor - inconvenience | misleading | easy workaround
  • User visibility set to Operational - other Techniques | Technique editor | Rudder settings
  • Priority set to 0
Actions
Copy link
 #33

Updated by Vincent MEMBRÉ about 7 years ago

  • Target version changed from 3.1.19 to 3.1.20
Actions
Copy link
 #34

Updated by Vincent MEMBRÉ almost 7 years ago

  • Target version changed from 3.1.20 to 3.1.21
Actions
Copy link
 #35

Updated by François ARMAND almost 7 years ago

The behavior is the expected one: id can not be modified. So we should just return an error here.

Actions
Copy link
 #36

Updated by François ARMAND almost 7 years ago

  • Priority changed from 0 to 14
Actions
Copy link
 #37

Updated by Vincent MEMBRÉ almost 7 years ago

  • Target version changed from 3.1.21 to 3.1.22
Actions
Copy link
 #38

Updated by Benoît PECCATTE almost 7 years ago

  • Priority changed from 14 to 27
Actions
Copy link
 #39

Updated by Vincent MEMBRÉ over 6 years ago

  • Target version changed from 3.1.22 to 3.1.23
Actions
Copy link
 #40

Updated by Vincent MEMBRÉ over 6 years ago

  • Target version changed from 3.1.23 to 3.1.24
Actions
Copy link
 #41

Updated by Vincent MEMBRÉ over 6 years ago

  • Target version changed from 3.1.24 to 3.1.25
  • Priority changed from 27 to 28
Actions
Copy link
 #42

Updated by Vincent MEMBRÉ over 6 years ago

  • Target version changed from 3.1.25 to 387
Actions
Copy link
 #43

Updated by Vincent MEMBRÉ over 6 years ago

  • Target version changed from 387 to 4.1.10
Actions
Copy link
 #44

Updated by Vincent MEMBRÉ about 6 years ago

  • Target version changed from 4.1.10 to 4.1.11
  • Priority changed from 28 to 29
Actions
Copy link
 #45

Updated by Vincent MEMBRÉ about 6 years ago

  • Target version changed from 4.1.11 to 4.1.12
Actions
Copy link
 #46

Updated by Vincent MEMBRÉ almost 6 years ago

  • Target version changed from 4.1.12 to 4.1.13
Actions
Copy link
 #47

Updated by Vincent MEMBRÉ almost 6 years ago

  • Target version changed from 4.1.13 to 4.1.14
  • Priority changed from 29 to 30
Actions
Copy link
 #48

Updated by Benoît PECCATTE over 5 years ago

  • Target version changed from 4.1.14 to 4.1.15
Actions
Copy link
 #49

Updated by Vincent MEMBRÉ over 5 years ago

  • Target version changed from 4.1.15 to 4.1.16
Actions
Copy link
 #50

Updated by Vincent MEMBRÉ over 5 years ago

  • Target version changed from 4.1.16 to 4.1.17
  • Priority changed from 30 to 31
Actions
Copy link
 #51

Updated by Vincent MEMBRÉ over 5 years ago

  • Target version changed from 4.1.17 to 4.1.18
  • Priority changed from 31 to 0
Actions
Copy link
 #52

Updated by Vincent MEMBRÉ over 5 years ago

  • Target version changed from 4.1.18 to 4.1.19
Actions
Copy link
 #53

Updated by Alexis Mousset about 5 years ago

  • Target version changed from 4.1.19 to 4.1.20
Actions
Copy link
 #54

Updated by Alexis Mousset almost 5 years ago

  • Target version changed from 4.1.20 to 588
Actions
Copy link
 #55

Updated by Alexis Mousset almost 5 years ago

  • Target version changed from 588 to 5.0.13

Very likely still present

Actions
Copy link
 #56

Updated by Vincent MEMBRÉ over 4 years ago

  • Target version changed from 5.0.13 to 5.0.14
Actions
Copy link
 #57

Updated by Vincent MEMBRÉ over 4 years ago

  • Target version changed from 5.0.14 to 5.0.15
Actions
Copy link
 #58

Updated by François ARMAND over 4 years ago

  • Subject changed from Modifying id of a parameter with API does not work but return a succes an launch a promise generation to id of a parameter can not be modified so API should return an error in that case
  • Effort required set to Very Small

As the behavior is the expected, we just need to return an error.

Actions
Copy link
 #59

Updated by Elaad FURREEDAN over 4 years ago

  • Status changed from New to In progress
  • Assignee set to Elaad FURREEDAN
Actions
Copy link
 #60

Updated by François ARMAND over 4 years ago

  • Subject changed from id of a parameter can not be modified so API should return an error in that case to Non used parameters in an API call should lead to an error
  • Target version changed from 5.0.15 to 6.1.0~beta1
  • Effort required changed from Very Small to Large

There two kinds of error here:

- the user tried to modify an attribute that was returned in the success message. He tried something not in the doc, but the API should let him knows that in that case, he will not be modified.
- the user just added an attribute that doesn't exists. In that case, it's extremely likely that the user didn't do what he wanted, tipycally he wrote "decsription" in place of "description". We should make it an error.

But we also want to have a "lenient" mode, for compatibility (or migration), where we don't check for non existing attributes.
It is a BIG change, one that we need to handle with schema at JSON level and autogenerate in some way, else it will be unmaintainable.

It also only doable in a major version (so the sooner possible is 6.1)

Actions
Copy link
 #61

Updated by François ARMAND over 4 years ago

  • Status changed from In progress to New
  • Assignee deleted (Elaad FURREEDAN)
Actions
Copy link
 #62

Updated by Vincent MEMBRÉ almost 4 years ago

  • Target version changed from 6.1.0~beta1 to 6.1.0~beta2
Actions
Copy link
 #63

Updated by Vincent MEMBRÉ almost 4 years ago

  • Target version changed from 6.1.0~beta2 to 6.1.0~beta3
Actions
Copy link
 #64

Updated by François ARMAND almost 4 years ago

  • Target version changed from 6.1.0~beta3 to 6.2.0~beta1
Actions
Copy link
 #65

Updated by Vincent MEMBRÉ over 3 years ago

  • Target version changed from 6.2.0~beta1 to 6.2.0~rc1
Actions
Copy link
 #66

Updated by François ARMAND over 3 years ago

  • Target version deleted (6.2.0~rc1)
Actions
Copy link
 #67

Updated by François ARMAND about 2 years ago

  • Tracker changed from Bug to Architecture
  • Severity deleted (Minor - inconvenience | misleading | easy workaround)
  • User visibility deleted (Operational - other Techniques | Technique editor | Rudder settings)
  • Priority deleted (0)

Actually, making an error for non existing parameter would significantly increase pain in migration scenario, where you can't update your scripts with new fields first (which are ignored) and change rudder after.

Plus it would a major breaking change.

I think it would have been better as a default if we had used it from the start, but now, it would be a big architecture change, and it's not a big query from users => changing tracker to arch.

Actions
Copy link
 #68

Updated by François ARMAND 8 months ago

  • Status changed from New to Rejected
  • Regression set to No

We don't want to do that to ease migration.

Actions
Copy link

Also available in: Atom PDF