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

PMM-12376 single inventory add endpoint #2422

Merged
merged 34 commits into from
Sep 13, 2023

Conversation

ademidoff
Copy link
Member

@ademidoff ademidoff commented Aug 20, 2023

PMM-12376

Link to the Feature Build: SUBMODULES-3366

If this PR adds or removes or alters one or more API endpoints, please review and add or update the relevant API documents as well:

This is how the deprecation of endpoints looks like:
Screenshot 2023-08-21 at 08 57 52

These are the endpoint params:
Screenshot 2023-08-25 at 16 38 20

@codecov
Copy link

codecov bot commented Aug 20, 2023

Codecov Report

Merging #2422 (eabc9d8) into main (3c58ed9) will increase coverage by 0.08%.
The diff coverage is 79.16%.

@@            Coverage Diff             @@
##             main    #2422      +/-   ##
==========================================
+ Coverage   42.71%   42.80%   +0.08%     
==========================================
  Files         384      384              
  Lines       48194    48227      +33     
==========================================
+ Hits        20587    20642      +55     
+ Misses      25668    25640      -28     
- Partials     1939     1945       +6     
Flag Coverage Δ
admin 10.45% <ø> (+0.04%) ⬆️
agent 52.97% <ø> (-0.16%) ⬇️
vmproxy 69.69% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
managed/services/inventory/grpc/nodes_server.go 0.00% <0.00%> (ø)
managed/services/inventory/nodes.go 69.49% <82.60%> (+11.84%) ⬆️

... and 6 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ademidoff ademidoff marked this pull request as ready for review August 21, 2023 10:08
@ademidoff ademidoff requested review from a team and BupycHuk as code owners August 21, 2023 10:08
@ademidoff ademidoff requested review from idoqo and JiriCtvrtka and removed request for a team August 21, 2023 10:08
Comment on lines +109 to +112
func (s *nodesServer) AddNode(ctx context.Context, req *inventorypb.AddNodeRequest) (*inventorypb.AddNodeResponse, error) {
return s.svc.AddNode(ctx, req)
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Minor: I think we can remove this file at all. Some time ago we agreed that these grpc packages are redundant.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, we should remove all the three in one shot IMO. Will do as a separate effort.

@artemgavrilov
Copy link
Contributor

Also API tests are missing

@ademidoff ademidoff marked this pull request as draft August 25, 2023 09:21
@ademidoff
Copy link
Member Author

Also API tests are missing

Thanks for pointing it out. Done now 🎉

@ademidoff ademidoff marked this pull request as ready for review August 28, 2023 07:05
@ademidoff ademidoff enabled auto-merge (squash) September 12, 2023 14:01
@ademidoff ademidoff disabled auto-merge September 12, 2023 14:06
@ademidoff ademidoff enabled auto-merge (squash) September 13, 2023 11:24
@ademidoff ademidoff merged commit df3b1aa into main Sep 13, 2023
@ademidoff ademidoff deleted the PMM-12376-single-inventory-add-endpoint branch September 13, 2023 11:44
ademidoff pushed a commit that referenced this pull request Sep 16, 2023
* PMM-12376 add a single NodeAdd endpoint

* PMM-12376 add a couple tests for AddNode

* PMM-12376 add more tests

* PMM-12376 remove debug lines

* PMM-12376 format the code

* PMM-12376 address linter errors

* PMM-12376 pass context to transactions

* PMM-12376 add more tests

* PMM-12376 clean up the tests

* PMM-12376 improve the tests

* PMM-12376 re-generate the specs

* PMM-12376 add AddNode to grpc server

* PMM-12376 update the APi documentation

* PMM-12376 update the documentation

* PMM-12376 prefer oneof vs optional

* PMM-12376 update the docs

* PMM-12376 add API tests

* PMM-12376 fix formatting

* PMM-12376 fix API nodes_test

* PMM-12376 fix API tests

* PMM-12376 reindex message props in nodes.proto

Co-authored-by: Artem Gavrilov <[email protected]>

* PMM-12376 use NotEmpty for checking an array

Co-authored-by: Artem Gavrilov <[email protected]>

* PMM-12376 move back the api-tests/server tests

* PMM-12376 running make gen

* PMM-12376 replace NotZerof with NotEmptyf

* PMM-12376 fix the TestContainerNode/Basic API test

---------

Co-authored-by: Artem Gavrilov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants