-
Notifications
You must be signed in to change notification settings - Fork 14
CLOUDP-322242: Add Service account curl example to OAS #742
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
Conversation
{ | ||
Lang: "cURL", | ||
Label: "curl", | ||
Label: "curl (Service Account)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all test.go files were updated for e2e tests
|
||
echo "Running FOAS CLI versions command" | ||
foascli versions -s openapi-foas.json -o ./openapi/v2/versions.json --env "${target_env:?}" --stability-level stable --stability-level preview | ||
foascli versions -s openapi-foas.json -o ./openapi/v2/versions.json --env "${target_env:?}" --stability-level stable --stability-level preview --stability-level upcoming |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
drive by fix to list upcoming api versions in the version.json
file
f.newServiceAccountCurlCodeSamplesForOperation(pathName, opMethod), | ||
f.newDigestCurlCodeSamplesForOperation(pathName, opMethod), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the main change is here
|
||
return codeSample{ | ||
Lang: "cURL", | ||
Label: "curl (Service Account Access Token)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could the label be shorter but still helpful?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CC @gssbzn since he proposed the name (before it was Service Account
). Is Service Account
too generic? Could Service Account
mean something else than Access Token
in this context?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i was about to propose that :-) if you see the screenshot in the PR description, it seems too long, but ok if we think this is the shortest label that makes it understable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see in the Spec we have Service accounts (oauth2)
(https://www.mongodb.com/docs/api/doc/atlas-admin-api-v2/) so I can use Service accounts - oauth2
which is shorter but still long.
I am going to update it to Service accounts - oauth2
for consistency and see if it is still too long.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I asked our PM for his preference in https://mongodb.slack.com/archives/C02FHQ614UR/p1749032541122489. Let's wait and see
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my fear with just service account is that you don't use the service account you need to get an access token, it would be good if we could link to how to get the access token but I'm not sure if that's possible on examples
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
works for me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Use the name "Service Accounts" for curl examples
- Include the AtlasCLI example first in the list
1) Use the name "Service Accounts" for curl examples 2) Include the AtlasCLI example first in the list
Proposed changes
CLOUDP-322242
Description
This PR adds curl request examples using Service account to all the endpoints.