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

DOCS-2475: Add additional headers #3141

Merged
merged 7 commits into from
Jul 29, 2024
Merged

Conversation

npentrel
Copy link
Collaborator

No description provided.

@viambot viambot added the safe to build This pull request is marked safe to build from a trusted zone label Jul 18, 2024
| `Robot-ID` | The machine that triggered the request. |
| `Location-ID` | The location of the machine that triggered the request. |
| `Org-ID` | The organization that triggered the request. |
| `Org-Id` | The ID of the organization that triggered the request. |
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@maxhorowitz : I've not been able to confirm these all work:

Here is an example of a trigger from ingested binary data. I don't get the associated component type, name, method name, and min-time received, max time received.

127.0.0.1 - - [18/Jul/2024 17:49:42] "POST /?data_type=binary&event_type=part_data_ingested HTTP/1.1" 200 -
ImmutableMultiDict([('data_type', 'binary'), ('event_type', 'part_data_ingested')])
Host: d99b-71-167-222-76.ngrok-free.app
User-Agent: Go-http-client/2.0
Content-Length: 375
Accept-Encoding: gzip
Content-Type: application/json
Location-Id: vw3iu72d8n
Org-Id: bccf8f8f-e3c4-4f72-ab9a-fc547757f352
Part-Id: 422afb68-21d0-487c-a135-01d81ff54407
Robot-Id: 584b3120-0da5-4014-b843-8a685e9cab2d
X-Forwarded-For: 2600:1900:2000:a3::1:900
X-Forwarded-Host: d99b-71-167-222-76.ngrok-free.app
X-Forwarded-Proto: https

Copy link
Collaborator Author

@npentrel npentrel Jul 18, 2024

Choose a reason for hiding this comment

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

And then here's an example of part_online, why is it sending Org-ID and Part-ID rather than the actual values?

127.0.0.1 - - [18/Jul/2024 17:46:08] "GET /?event_type=part_online HTTP/1.1" 200 -
ImmutableMultiDict([('event_type', 'part_online')])
Host: d99b-71-167-222-76.ngrok-free.app
User-Agent: Go-http-client/2.0
Accept: application/json
Accept-Encoding: gzip
Content-Type: application/json
Location-Id: vw3iu72d8n
Location-Name: First Location
Machine-Name: guardian
Org-Id: Org-ID
Organization-Name: Robot Land
Part-Id: Part-ID
Part-Name: guardian-main
Robot-Id: 584b3120-0da5-4014-b843-8a685e9cab2d
X-Forwarded-For: 2600:1900:2000:a6::1:901
X-Forwarded-Host: d99b-71-167-222-76.ngrok-free.app
X-Forwarded-Proto: https

Copy link
Member

Choose a reason for hiding this comment

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

1st message -

component type, name, method name, min-time received, and max time received were moved to the data ingested POST request body - I am sorry if I grazed over this in a previous review - it was a change that was made during the introduction of POST request for part data ingested webhooks
Screenshot 2024-07-22 at 4 35 59 PM

2nd message -

https://viam.atlassian.net/browse/APP-5608

Comment on lines 298 to 299
| `Org-Id` | The ID of the organization that triggered the request. | `part_data_ingested` |
| `Organization-Name` | The name of the organization that triggered the request. | `part_online`, `part_offline` |
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@maxhorowitz also why is this so awkward? Shouldn't they all return org-id, org name, location id, location name, etc?

Copy link
Member

Choose a reason for hiding this comment

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

TLDR is this is a bug - ticket is here - is this something you'd like prio'd ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No all good. Was just wondering if it had been an oversight. No urgency from my end



@app.route("/", methods=['GET', 'POST'])
def hello_http_get():
Copy link
Member

Choose a reason for hiding this comment

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

add post too?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh it'll do both but I can rename the function to not mention get 😅

Comment on lines 226 to 239
"Org-Id": headers.get('org-id', 'no value'),
"Organization-Name": headers.get('organization-name', 'no value'),
"Location-Id": headers.get('location-id', 'no value'),
"Location-Name": headers.get('location-name', 'no value'),
"Part-Id": headers.get('part-id', 'no value'),
"Part-Name": headers.get('part-name', 'no value'),
"Robot-Id": headers.get('robot-id', 'no value'),
"Machine-Name": headers.get('machine-name', 'no value'),
"Component-Type": headers.get('component-type', 'no value'),
"Component-Name": headers.get('component-name', 'no value'),
"Method-Name": headers.get('method-name', 'no value'),
"Min-Time-Received": headers.get('min-time-received', 'no value'),
"Max-Time-Received": headers.get('max-time-received', 'no value'),
"Data-Type": request.args.get('data_type', 'no value')
Copy link
Member

Choose a reason for hiding this comment

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

ComponentName, ComponentType, MethodName, MachineName, LocationName, OrgName, FileID, FilePath are each parsed from within the request's body, not headers

@maxhorowitz
Copy link
Member

FYI closing https://viam.atlassian.net/browse/APP-5608 today 👍

@npentrel npentrel requested a review from maxhorowitz July 26, 2024 15:09
@npentrel npentrel force-pushed the DOCS-2475 branch 3 times, most recently from f00e808 to 627dffd Compare July 26, 2024 15:23
| `Method-Name` | The name of the method from which data was ingested. Only for `part_data_ingested` triggers. |
| `Min-Time-Received` | Indicates the earliest time a piece of data was received. |
| `Max-Time-Received` | Indicates the latest time a piece of data was received. |
| Header Key | Description | Trigger types |
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@maxhorowitz are these the correct trigger types for each header key?

@viambot
Copy link
Member

viambot commented Jul 26, 2024

You can view a rendered version of the docs from this PR at https://docs-test.viam.dev/3141

Copy link
Member

@maxhorowitz maxhorowitz left a comment

Choose a reason for hiding this comment

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

Cross checked with the code and everything seems correct to me - as long as you've manually tested that lambda against a trigger we're good to merge! Thank you

@npentrel
Copy link
Collaborator Author

yep all tested

@npentrel npentrel merged commit df07fac into viamrobotics:main Jul 29, 2024
8 checks passed
@npentrel npentrel deleted the DOCS-2475 branch July 29, 2024 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
safe to build This pull request is marked safe to build from a trusted zone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants