Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify success response for telemetry update #461

Merged
merged 2 commits into from
Jun 26, 2020
Merged

Clarify success response for telemetry update #461

merged 2 commits into from
Jun 26, 2020

Conversation

Retzoh
Copy link
Contributor

@Retzoh Retzoh commented Mar 4, 2020

MDS Pull Request

Thank you for your contribution!

For most Pull Requests, the target branch should be dev. Please ensure you are targeting dev to avoid complications and help make the Review process as smooth as possible.

Explain pull request

The telemetry spec only says that when new telemetry are successfully received, the server should:

Responds with number of successfully written telemetry data points and total number of provided points.

But the spec does not specify how those two numbers should be written.

Is this a breaking change

A breaking change would require consumers or implementors of the API to modify their code for it to continue to function (ex: renaming of a required field or the change in data type of an existing field). A non-breaking change would allow existing code to continue to function (ex: addition of an optional field or the creation of a new optional endpoint).

  • Yes, breaking

Impacted Spec

Which spec(s) will this pull request impact?

  • agency

Additional context

None

The telemetry spec only says that when new telemetry are successfully received, the server should:
> Responds with number of successfully written telemetry data points and total number of provided points.

But the format is not specified.
@Retzoh Retzoh requested a review from a team March 4, 2020 09:39
@Retzoh Retzoh requested a review from a team as a code owner March 4, 2020 09:39
@Retzoh
Copy link
Contributor Author

Retzoh commented Mar 4, 2020

This PR would be rendered obsolete by #462

@sarob sarob added this to the Future milestone Mar 8, 2020
@sarob sarob added Agency Specific to the Agency API enhancement New feature or request labels Mar 8, 2020
@jfh01 jfh01 modified the milestones: Future, 1.0.0 Apr 9, 2020
@schnuerle
Copy link
Member

Waiting to merge to see if we move forward on #462 for the 1.0.0. It needs some decisions and a PR.

agency/README.md Outdated
@@ -209,7 +209,7 @@ Body Params:

| Field | Type | Field Description |
| --------- | ------------------------------ | ------------------------------------------------------------------------------------------------------- |
| `result` | String | Responds with number of successfully written telemetry data points and total number of provided points. |
| `result` | String | Responds with number of successfully written telemetry data points and total number of provided points. Format to use: `{success}/{total}`, both amounts as integers. |
Copy link
Collaborator

Choose a reason for hiding this comment

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

How about adding successes and total as integers, in addition to a result string, or dropping result entirely?

- Change response code to 200 to solve issue #461
- Split `result` response field in `success` and `total`
@Retzoh
Copy link
Contributor Author

Retzoh commented Jun 25, 2020

@karcass @schnuerle I just updated the PR, you can review it

@Retzoh Retzoh linked an issue Jun 25, 2020 that may be closed by this pull request
@schnuerle schnuerle merged commit 19a878e into openmobilityfoundation:dev Jun 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Agency Specific to the Agency API enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Agency: telemetry endpoint returns a success in case of failures
5 participants