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.

Sign up for GitHub

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

Jump to bottom

feat(specs): add OpenAPI spec for Monitoring API #1683

Merged
merged 38 commits into from
Jul 11, 2023
Merged

feat(specs): add OpenAPI spec for Monitoring API #1683

merged 38 commits into from
Jul 11, 2023

Conversation

kai687
Copy link
Contributor

@kai687 kai687 commented Jul 3, 2023 •
edited by shortcuts
Loading

🧭 What and Why

Create an OpenAPI specification for the Monitoring API.
The Monitoring API is HTTP only and isn't included in the API clients.

🎟 JIRA Ticket: DOC-1132 https://algolia.atlassian.net/browse/DI-1419

@netlify
Copy link

netlify bot commented Jul 3, 2023 •
edited
Loading

✅ Deploy Preview for api-clients-automation ready!

Name Link
🔨 Latest commit d9b9e2d
🔍 Latest deploy log https://app.netlify.com/sites/api-clients-automation/deploys/64ad2472f85bdf0007894234
😎 Deploy Preview https://deploy-preview-1683--api-clients-automation.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@algolia-bot
Copy link
Collaborator

algolia-bot commented Jul 3, 2023 •
edited
Loading

✗ The generated branch has been deleted.

If the PR has been merged, you can check the generated code on the main branch.
You can still access the code generated on main via this commit.

@kai687 kai687 changed the title feat: add OpenAPI spec for Monitoring API feat(specs): add OpenAPI spec for Monitoring API Jul 4, 2023
@kai687 kai687 requested a review from gazconroy July 4, 2023 06:28
@kai687 kai687 marked this pull request as ready for review July 4, 2023 06:28
@kai687 kai687 requested a review from a team as a code owner July 4, 2023 06:28
@kai687 kai687 requested review from millotp and shortcuts July 4, 2023 06:28
@kai687
Copy link
Contributor Author

kai687 commented Jul 4, 2023

@shortcuts The Monitoring API doesn't have API clients, so I'm not sure how to configure this project to just use this spec for documentation. Can you help me?

@millotp
Copy link
Collaborator

millotp commented Jul 4, 2023

Hey, is it bad if the Monitoring API client is generated ? Or is it something that should not be generated ?

@shortcuts
Copy link
Member

shortcuts commented Jul 4, 2023 •
edited
Loading

@kai687 I've added the required stuff to make the CI and local tooling recognize the monitoring spec, it might not be enough to do the generation but as of now it already runs the build and linter steps on the specs, so that's great to progress on writing it! you can do yarn docker build specs monitoring locally

@shortcuts
Copy link
Member

shortcuts commented Jul 4, 2023 •
edited
Loading

Hey, is it bad if the Monitoring API client is generated ? Or is it something that should not be generated ?

Today there's none, but the API is documented and public, so we can generate it

gazconroy
gazconroy requested changes Jul 4, 2023
View reviewed changes
Copy link
Contributor

@gazconroy gazconroy left a comment

Choose a reason for hiding this comment

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

A few suggestions and a question

specs/monitoring/common/responses/InfrastructureResponse.yml Outdated Show resolved Hide resolved
specs/monitoring/common/responses/InfrastructureResponse.yml Outdated Show resolved Hide resolved
specs/monitoring/common/schemas/Incidents.yml Outdated Show resolved Hide resolved
specs/monitoring/common/parameters.yml Outdated Show resolved Hide resolved
specs/monitoring/paths/getInventory.yml Outdated Show resolved Hide resolved
specs/monitoring/paths/getReachability.yml Outdated Show resolved Hide resolved
specs/monitoring/paths/getStatus.yml Outdated Show resolved Hide resolved
kai687 and others added 8 commits July 4, 2023 12:32
@kai687 @gazconroy
Apply suggestions from code review
0d5a269 
Co-authored-by: gazconroy <gazconroyster@gmail.com>
@kai687 @gazconroy
Apply suggestions from code review
01c0ec2 
Co-authored-by: gazconroy <gazconroyster@gmail.com>
@kai687
fix: spec for docs
49405b1
@kai687
fix: throw errors when wrong tags
8e1f8bc 
Instead of silently returning when a tag name is the same as
the client name or alias, throw errors. For example, we have the
monitoring API and the monitoring tag within that API.
Display names can be equal, but not the tag names themselves.
@kai687
fix: spec for docs
0f38136
@kai687
fix: parameters
789d099
@kai687
fix: clarify
d47390f
@shortcuts
fix: textual form for globs
9439820
@shortcuts
Copy link
Member

@kai687 I found the issue for the java generation, this should be good now!

@shortcuts
Copy link
Member

@kai687 the CI fails because the tests I wrote were before the rename of the methods

@kai687 kai687 requested a review from shortcuts July 10, 2023 09:56
@shortcuts shortcuts mentioned this pull request Jul 10, 2023
Merged
@shortcuts
Copy link
Member

ok wow I found a looooot of bugs in this PR, so glad you did it and sorry for all the commits :D

I'm moving unrelated changes to its own PR, then doing a cleanup

@shortcuts shortcuts mentioned this pull request Jul 10, 2023
Merged
shortcuts
shortcuts approved these changes Jul 11, 2023
View reviewed changes
Copy link
Member

@shortcuts shortcuts left a comment

Choose a reason for hiding this comment

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

wonderful!! and this PR only solved like 3 bugs, so... thanks a lot for the contribution!

millotp
millotp reviewed Jul 11, 2023
View reviewed changes
clients/algoliasearch-client-javascript/yarn.lock
@@ -109,6 +109,19 @@ __metadata:
languageName: unknown
linkType: soft

"@algolia/monitoring@workspace:packages/monitoring":
Copy link
Collaborator

Choose a reason for hiding this comment

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

isn't that file generated ?

scripts/ci/utils.ts Show resolved Hide resolved
@shortcuts shortcuts merged commit 8765f6d into main Jul 11, 2023
@shortcuts shortcuts deleted the feat/specs/monitoring branch July 11, 2023 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shortcuts shortcuts shortcuts approved these changes

@millotp millotp millotp left review comments

@aallam aallam aallam left review comments

@gazconroy gazconroy Awaiting requested review from gazconroy

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@kai687 @algolia-bot @millotp @shortcuts @aallam @gazconroy