camelCase and snake_case are mixed in the admin API

quentin commented

2022-09-13 14:41:36 +00:00

Owner

In the admin API, camel case and snake case are mixed, for example on /status:

{
  "node": "8bd56738f33dd833e30ae51d900ed883df06a2b169f830d01a8858f2b9e70db4",
  "garageVersion": "v0.7.3",
  "knownNodes": {
    "8bd56738f33dd833e30ae51d900ed883df06a2b169f830d01a8858f2b9e70db4": {
      "addr": "[::1]:3901",
      "is_up": true,
      "last_seen_secs_ago": 4,
      "hostname": "rincevent"
    }
  },
  "layout": {
    "version": 0,
    "roles": {},
    "stagedRoleChanges": {}
  }
}

garageVersion is in camelcase
is_up is in snake case

I would say that snake case is more idiomatic for JSON.
Could we convert everything to snake_case? As it is a breaking change, can we do this at the same time we handle other tickets, like #378?

Also the doc is incorrect as it says garage_version and not garageVersion.
https://garagehq.deuxfleurs.fr/documentation/reference-manual/admin-api/

In the admin API, camel case and snake case are mixed, for example on `/status`: ```json { "node": "8bd56738f33dd833e30ae51d900ed883df06a2b169f830d01a8858f2b9e70db4", "garageVersion": "v0.7.3", "knownNodes": { "8bd56738f33dd833e30ae51d900ed883df06a2b169f830d01a8858f2b9e70db4": { "addr": "[::1]:3901", "is_up": true, "last_seen_secs_ago": 4, "hostname": "rincevent" } }, "layout": { "version": 0, "roles": {}, "stagedRoleChanges": {} } } ``` - `garageVersion` is in camelcase - `is_up` is in snake case I would say that snake case is more idiomatic for JSON. Could we convert everything to snake_case? As it is a breaking change, can we do this at the same time we handle other tickets, like #378? Also the doc is incorrect as it says `garage_version` and not `garageVersion`. https://garagehq.deuxfleurs.fr/documentation/reference-manual/admin-api/

quentin added the

labels 2022-09-13 14:41:36 +00:00

lx commented

2022-09-14 11:08:02 +00:00

Owner

Normally everything is camelCase. Anything that is not is a bug and should be fixed. Here is an example of how to make a struct camelCase when it is serialized:

#[derive(Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
struct KeyPerm {
	#[serde(default)]
	create_bucket: bool,
}

Normally everything is camelCase. Anything that is not is a bug and should be fixed. Here is an example of how to make a struct camelCase when it is serialized: ```rust #[derive(Serialize, Deserialize)] #[serde(rename_all = "camelCase")] struct KeyPerm { #[serde(default)] create_bucket: bool, } ```

quentin commented

2022-09-14 13:21:48 +00:00

Author

Owner

Do we really want to rename everything in camel case?
Serde by default generate snake case on JSON and snake case is a standard among JSON REST API. For example, the Twitter API uses snake case JSON: https://developer.twitter.com/en/docs/twitter-api/v1/data-dictionary/overview

What are the benefit of camel case in our context?

In the other way, forgetting to put a "rename tag" is very easy: do we really want to add this complexity in the development? I am sure that it will be forgotten for some other structures in the future.

Do we really want to rename everything in camel case? Serde by default generate snake case on JSON and snake case is a standard among JSON REST API. For example, the Twitter API uses snake case JSON: https://developer.twitter.com/en/docs/twitter-api/v1/data-dictionary/overview What are the benefit of camel case in our context? In the other way, forgetting to put a "rename tag" is very easy: do we really want to add this complexity in the development? I am sure that it will be forgotten for some other structures in the future.

lx commented

2022-09-14 13:22:54 +00:00

Owner

I don't mind changing to snake_case in theory. In practice it might break some user's workflows?

quentin commented

2022-09-14 13:25:38 +00:00

Author

Owner

I think we should keep the admin API in its current state for garage v0.8.x, collecting as much feedback as possible, and release a v1 fixing all the issues we referenced for v0.9.0 :-)

I think we should keep the admin API in its current state for garage v0.8.x, collecting as much feedback as possible, and release a `v1` fixing all the issues we referenced for v0.9.0 :-)

quentin added this to the v0.9 milestone 2022-09-14 13:26:23 +00:00

lx referenced this issue from a commit

2023-06-13 14:16:17 +00:00

admin API: add missing camelCase conversions (fix #381)

lx referenced this issue from a commit

2023-06-14 11:53:41 +00:00

admin api: switch GetClusterHealth to camelcase (fix #381 again)

lx commented

2023-06-14 12:21:31 +00:00

Owner

Fixed by multiple commits (bf19a44fd9, 35c108b85d) in branch next, which will be merged for Garage v0.9

The choice was made to stick to camelCase everywhere (which is fairly common in many json protocols, e.g. JSON-LD/ActivityPub)

Fixed by multiple commits (bf19a44fd9, 35c108b85d) in branch `next`, which will be merged for Garage v0.9 The choice was made to stick to camelCase everywhere (which is fairly common in many json protocols, e.g. JSON-LD/ActivityPub)

lx closed this issue

2023-06-14 12:21:31 +00:00

lx referenced this issue

2023-09-04 09:39:18 +00:00

Garage v0.9 #473

camelCase and snake_case are mixed in the admin API #381