Skip to content

Chore/api reference scripts delete#444

Merged
sunbryely-influxdata merged 13 commits intomasterfrom
chore/api-reference-scripts-delete
Nov 4, 2022
Merged

Chore/api reference scripts delete#444
sunbryely-influxdata merged 13 commits intomasterfrom
chore/api-reference-scripts-delete

Conversation

@sunbryely-influxdata
Copy link
Copy Markdown
Contributor

Delete script
Get script
Patch script

@sunbryely-influxdata sunbryely-influxdata marked this pull request as ready for review July 19, 2022 23:38
@sunbryely-influxdata sunbryely-influxdata requested a review from a team as a code owner July 19, 2022 23:38
@sunbryely-influxdata sunbryely-influxdata requested review from jstirnaman and sanderson and removed request for a team July 19, 2022 23:38
Comment thread src/svc/invocable-scripts/paths/scripts_scriptID.yml Outdated
Comment thread src/svc/invocable-scripts/paths/scripts_scriptID.yml Outdated
Comment thread src/svc/invocable-scripts/paths/scripts_scriptID.yml Outdated
Comment thread src/svc/invocable-scripts/paths/scripts_scriptID.yml Outdated
Comment thread src/svc/invocable-scripts/paths/scripts_scriptID.yml Outdated
Comment thread src/svc/invocable-scripts/schemas/ScriptUpdateRequest.yml Outdated
Comment thread src/svc/invocable-scripts/paths/scripts_scriptID.yml Outdated
Comment thread src/svc/invocable-scripts/paths/scripts_scriptID.yml Outdated
Comment thread src/svc/invocable-scripts/schemas/ScriptUpdateRequest.yml
Comment thread src/svc/invocable-scripts/paths/scripts_scriptID.yml Outdated
Comment thread src/svc/invocable-scripts/paths/scripts_scriptID.yml Outdated
Comment thread src/svc/invocable-scripts/paths/scripts_scriptID.yml Outdated
Updates properties (`name`, `description`, and `script`) of an invokable script.
Updates an invokable script.

Use this endpoint to update the properties (`description`, and `script`)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Use this endpoint to update the properties (`description`, and `script`)
Use this endpoint to modify values for script properties (`description` and `script`).

Updates an invokable script.

Use this endpoint to update the properties (`description`, and `script`)
of an invokable script.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
of an invokable script.


#### Limitations

- Sending an empty request body returns an HTTP `200` status code.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Sending an empty request body returns an HTTP `200` status code.
- If you send an empty request body, InfluxDB responds with an HTTP `200` status code.

examples:
notFound:
summary: |
The requested script was not found.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The requested script was not found.
The requested script wasn't found.


#### Limitations

- Only one script can be deleted per request.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Only one script can be deleted per request.
- You can delete only one script per request.

#### Limitations

- Only one script can be deleted per request.
- `scriptIDs` that do not exist return an HTTP `204` status code.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- `scriptIDs` that do not exist return an HTTP `204` status code.
- If the script ID you provide doesn't exist for the organization, InfluxDB responds with an HTTP `204` status code.

Copy link
Copy Markdown
Contributor

@jstirnaman jstirnaman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix and a question

Comment on lines +20 to +21
Script ID.
Only returns the script with this ID.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Script ID.
Only returns the script with this ID.
The ID of the script to retrieve.

I suppose we could apply that same pattern (like you did here) to all the GET ...${ID} operations. Makes things easier for us and is more descriptive.
I've following Square's convention with "The ID of the...".
What do you think? We should still lead with the article, though: "The script ID."

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think i like how square is wording it. "The ID of the..." sounds good to me

#### Limitations

- You can delete only one script per request.
- If the script ID you provide doesnt exist for the organization, InfluxDB
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- If the script ID you provide doesnt exist for the organization, InfluxDB
- If the script ID you provide doesn't exist for the organization, InfluxDB

required: true
description: The ID of the script to delete.
description: |
Script ID.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Script ID.
The script ID.

Updates properties (`name`, `description`, and `script`) of an invokable script.
Updates an invokable script.

Use this endpoint to modify values for script properties (`description`, and `script`).
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Use this endpoint to modify values for script properties (`description`, and `script`).
Use this endpoint to modify values for script properties (`description` and `script`).


#### Limitations

- If you send an empty request body, InfluxDB responds with an HTTP `200`
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this store an empty script?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not store an empty script. i'll add an additional statement saying that it does not store an empty script / does not update the script

#### Limitations

- You can delete only one script per request.
- If the script ID you provide doesn't exist for the organization, InfluxDB
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

description: The requested script object.
description: |
Success.
The response body contains the script object.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The response body contains the script object.
The response body contains the script detail.


#### Limitations

- If you send an empty request body, the script will neither update nor
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't create a script object with an empty script?

requestBody:
description: Script update to apply
description: |
An object that contains the updated script properties to apply.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My preference is to avoid mentioning "object" given that it's content-type (JSON) specific... even though it's the only content-type. I'm sure I break this rule occasionally.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

e.g. See how https://developers.google.com/youtube/v3/docs/playlistItems/insert?hl=en_US and https://developer.squareup.com/reference/square/payments-api/create-payment both avoid using content-type specific language. YouTube uses more HTTP/REST-ish descriptions like `resource' with a link to the schema entity. Square doesn't usually describe the payload as a thing, just the "Payment" and the fields.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think what you did is more practical, but maybe we could show it with an example instead of describe it.

required: true
description: The script ID.
description: |
Script ID.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Script ID.

source: |
curl -X 'PATCH' \
"https://cloud2.influxdata.com/api/v2/scripts/SCRIPT_ID" \
--header "Authorization: Token INFLUX_TOKEN" \
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
--header "Authorization: Token INFLUX_TOKEN" \
--header "Authorization: Token INFLUX_API_TOKEN" \

--header "Accept: application/json"
--header "Content-Type: application/json"
--data '{
"description": "get last point from new bucket",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"description": "get last point from new bucket",
"description": "Get the last point from the bucket",

Comment thread src/svc/invocable-scripts/schemas/ScriptUpdateRequest.yml
@sunbryely-influxdata sunbryely-influxdata merged commit 26b604e into master Nov 4, 2022
@sunbryely-influxdata sunbryely-influxdata deleted the chore/api-reference-scripts-delete branch November 4, 2022 23:11
Copy link
Copy Markdown
Contributor

@jstirnaman jstirnaman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎊

Use this endpoint to update the properties (`name`, `description`, and `script`) of an invokable script.
Use this endpoint to modify values for script properties (`description` and `script`).

To update a script, pass an object that contains the updated key-value pairs.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use the requestBody description for this and not duplicate.

Suggested change
To update a script, pass an object that contains the updated key-value pairs.

requestBody:
description: The script update to apply.
description: |
An object that contains the updated script properties to apply.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
An object that contains the updated script properties to apply.
In the request body, provide the script properties to update.

This is my new pattern. Eventually, I plan to append an statement (with redoc-cli) that introduces the properties that can be set, like in YouTube docs--"You can set the following properties for this resource:"

jstirnaman added a commit that referenced this pull request Nov 8, 2022
* 'master' of github.com:influxdata/openapi:
  chore: use /latest alias for OSS URL substitutions. (#603)
  fix(bucket schemas): `requestBody` for `createMeasurementSchema` and `updateMeasurementSchema` should be required (#607)
  fix(tasks): `orgID` should not be required for GetTasks (#606)
  chore(aim): Add Operator Entitlements Endpoints (#595)
  chore(aim): Add 403 to Org Creation Endpoints (#597)
  chore(invocable-scripts): add /api/v2/scripts/SCRIPT_ID/params (#593)
  Chore/api reference scripts delete (#444)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants