p3 initial changes #1177
p3 initial changes #1177
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@diana-macrometa @joubert-macrometa i will review the current api once more and if needed will make needful changes and then will remove the do not merge flag |
There was a problem hiding this comment.
- A few inconsistent 200 messages
- There are many options (fields) that have no description. I have no idea what to do with them? Not sure what values I should put in them or when they are reporting something, I don't know what these values mean.
- Most of these messages could use some help running through an LLM to help correct grammatic errors.
static/openapi/p3-spec.json
Outdated
| }, | ||
| "urlPattern": { | ||
| "type": "string", | ||
| "description": "Url pattern to group same pages under similar set of optimizations.", |
There was a problem hiding this comment.
The description says url. But the example is a path?
Is it a path or a URL?
There was a problem hiding this comment.
it is a path @joubert-macrometa
| "serverUrl": { | ||
| "type": "string", | ||
| "description": "Origin/server URL from where the html content needs to be fetched to run optimizations.", | ||
| "example": "something.com" |
There was a problem hiding this comment.
The description says URL, but the example is a host or a domain.
| }, | ||
| "responses": { | ||
| "200": { | ||
| "description": "PPM Job successfully created.", |
There was a problem hiding this comment.
In most return message you used "Successfully xxxx". Here it is different.
There was a problem hiding this comment.
Do not capitalize "job" here.
| "tags": [ | ||
| "PPM" | ||
| ], | ||
| "summary": "Get Web Vitals Metrics Percentile", |
There was a problem hiding this comment.
This sounds strange: "Vitals Metrics". Two plural words? Maybe not. Just noting it sounded funny to me.
There was a problem hiding this comment.
I do not like all the caps.
| "PPM" | ||
| ], | ||
| "summary": "Get Web Vitals Metrics Percentile", | ||
| "description": "Retrieves web vitals metrics percentile data based on the provided datetime range and pagination.", |
There was a problem hiding this comment.
I am not sure how this api works based on the parameters.
You pass in 75. What is it supposed to return?
There was a problem hiding this comment.
I would not bother mentioning the "pagination" in the description. Focus more on what is this API going to do for me? It looks like some kind of metrics, I don't know what vitals is, don't understand percentile data thing.
| ], | ||
| "responses": { | ||
| "200": { | ||
| "description": "Web vitals metrics percentile data successfully retrieved.", |
There was a problem hiding this comment.
Again different format from other 200 messages
There was a problem hiding this comment.
Joubert, is there guidance in your API directions? Because it sounds like there needs to be.
| "properties": { | ||
| "url": { | ||
| "type": "string", | ||
| "example": "https://something.com/something/" |
| "openapi": "3.0.0", | ||
| "info": { | ||
| "title": "PhotonIQ Performance Proxy API", | ||
| "description": "API documentation for the PhotonIQ P3 API Service", |
There was a problem hiding this comment.
| "description": "API documentation for the PhotonIQ P3 API Service", | |
| "description": "API documentation for PhotonIQ Performance Proxy (P3).", |
| }, | ||
| "serverUrl": { | ||
| "type": "string", | ||
| "example": "https://something.com" |
There was a problem hiding this comment.
@joubert-macrometa, do we have a standard example URL?
| "name": "Cache" | ||
| }, | ||
| { | ||
| "name": "PPM" |
There was a problem hiding this comment.
I don't like "PPM" as a section title, maybe spell it out? The acronym isn't obvious to users.
| }, | ||
| { | ||
| "name": "Telemetry", | ||
| "description": "(example can change)" |
There was a problem hiding this comment.
This and the next one need actual test, not placeholders.
| }, | ||
| "pageType": { | ||
| "type": "string", | ||
| "description": "Type of page to be optimized.", |
There was a problem hiding this comment.
Need to list the options.
| }, | ||
| "optimizationLevel": { | ||
| "type": "string", | ||
| "description": "Optimization category to decide which optimizations needs to be applied.", |
There was a problem hiding this comment.
The levels are light, standard, and aggressive. I still need those defined for the docs.
| }, | ||
| "optimizers": { | ||
| "type": "object", | ||
| "properties": { |
There was a problem hiding this comment.
Wait, the users can customize these from the API? They cannot do that from the UI, why the difference?
| "description": "Authorization failure due to invalid authentication credentials." | ||
| }, | ||
| "403": { | ||
| "description": "Either the API key doesn't have permissions or it is deactivated." |
There was a problem hiding this comment.
I thought it was JWT, not API key?
| "description": "Authorization failure due to invalid authentication credentials." | ||
| }, | ||
| "403": { | ||
| "description": "Either the API key doesn't have permissions or it is deactivated." |
There was a problem hiding this comment.
So, since JWTs are apparently an option, is it correct to have this error mention API keys throughout? Is it possible the user is authenticating with JWT instead?
Co-authored-by: Diana Payton <[email protected]>
Co-authored-by: Joubert Berger <[email protected]>
Co-authored-by: Joubert Berger <[email protected]>
Co-authored-by: Joubert Berger <[email protected]>
Co-authored-by: Diana Payton <[email protected]>
Co-authored-by: Diana Payton <[email protected]>
Co-authored-by: Diana Payton <[email protected]>
|
@borisgmacrometa, FYI, we are waiting for action on our edits for this PR. |
|
assigning this to @edgargarciamacrometa to make the changes in the pr as per the recommendations by @joubert-macrometa |
|
Changes are in this PR: #1231 |
No description provided.