Push changes for SystemLink 2020 R2 release#87
Merged
Conversation
…r them # Justification Need to generate swagger code and implement the generated interface to be used by the generated web service. # Implementation Added NuGet package dependency and generated code. Started implementing AssetPerformanceManagementWebService and implemented one of the simplest methods inside it. Adding tests. # Testing Ran all unit and integration tests. Copied the dlls in the local installation folder and made sure the product still works. # Checklist - [X] I tested changes to product code in product - [X] I considered updates to the wiki Related work items: #944460, #944461
…ement service Add permission-checks for PostJob, CancelJobs. This is supposed to be back-wards compatible, so the infrastructure should fall-back and use an
# Justification OpenAPIGenerator generates inline objects if we use undefined models. # Implementation Replaced inline objects with models in repo.yml. Added models for each inline object and inline response generated. Didn't change the API, only named all the used models. Named generated functions. # Testing Successfully generated the new classes by rebuilding the module.
# Justification Incorporates feedback from an open API review into the Test Monitor swagger doc. # Implementation - Fixes a typo in the min/max descriptions of `ValueSummary` - Notes that path IDs may not be stable after a repair - Make defaults/minimums for `take` consistent across all routes. Chose a common value set that doesn't change behavior anywhere, but provides the most accurate documentation. # Testing Verified the generated documentation was correct. # Checklist - [X] I tested changes to product code in product - [X] I considered updates to the wiki
# Implementation Added Error codes from the repo web service. Implemented 'C ' http handlers. # Testing Tested using swagger.
…e (part 4) # Implementation Implemented: - DeleteFeed - DeletePackage - GetPackage - GetFeed - GetStoreItem - ListPackages - QueryStoreItems Added: Model classes for Store Item handling Modified: yml models to contain the same info as the old web service # Testing Using swagger.
…art 5 # Justification Replace repo web service with generic web service. # Implementation Implemented http request handlers. # Testing Tested using swagger.
…comparison. # Justification We need to address the mismatches between the implementation and the specification of the routes. # Checklist - [ ] I tested changes to product code in product - [ ] I considered updates to the wiki
# Justification The SWAGGER doc says the "query-assets" route should work with "destination": "FILE_SERVICE" in the request body. It doesn't. # Implementation 1. Added EnumMember attribute to ResponseDestinationEnum 2. Updated the HttpController to return a better message than "MalformedJson" when the request body has an invalid combination of [ResponseFormat;Destination] 3. Updated the SWAGGER doc to describe what are the supported combinations of [ResponseFormat;Destination] # Testing 1. Validated from an HTTP Client 2. Create a unit test that verifies that the RequestCreationHelper correctly serializes "destination": "FILE_SERVICE". This is enough testing since the same RequestCreationHelper is used in our IntegrationTests (where the Asset service communicates with the FileIngestion service) # Checklist - [x] I tested changes to product code in product - [ ] I considered updates to the wiki Related work items: #949745
# Justification Implement Swagger generated interface. # Implementation Implement compare asset availability route/method as described in swagger doc. # Testing Ran all unit tests. # Checklist - [ ] I tested changes to product code in product - [ ] I considered updates to the wiki Related work items: #951066
Keep the same signature of the functions. # Implementation Changed validation methods. # Testing Tested all the implemented routes.
Add workspace to models and change routes. # Checklist - [ ] I tested changes to product code in product - [ ] I considered updates to the wiki
# Justification Implement Swagger generated interface # Implementation Implement delete assets method. Added a result object to be returned by the assets facade. This object is needed to be able to create the errors to be returned by the route. The object contains the ids of the deleted assets, the invalid ids and the not existing asset ids. # Testing Ran all unit tests. # Checklist - [ ] I tested changes to product code in product - [ ] I considered updates to the wiki Related work items: #947903
…outes # Justification Implement Swagger interfaces. # Implementation Implement the 2 methods and write tests. # Testing Ran all unit tests and validated in VM. # Checklist - [x] I tested changes to product code in product - [ ] I considered updates to the wiki Related work items: #952192
Added the workspace to the tag metadata so that tag path + workspace form now the unique key. Tags paths are only unique within their workspace. All create routes take an optional workspace field. If not specified, the default workspace is assumed which should allow us to preseve backwards compatibility. The query routes allow the caller to query across all workspaces they have access to. Routes which only have the tag path in the url will target the default workspace and I have added an "overload" for the route to take also theworkspace, e.g. - GET /nitag/v1/tags/my-tag-path --> returns the tag from the default workspace (if exists) - GET /nitag/v1/tags/workspace-x/my-tag-path --> searches the tag in workspace-x Selections and Subscriptions are only allowed within a single workspace.
# Justification Implement Swagger generated interface # Implementation Implement the get asset calibration history route. Added tests to cover all cases of success, bad request, resource not found and internal error. Moved 2 classes to infrastructure as they are used through multiple layers. # Testing Ran all unit tests and check in validation VM. # Checklist - [x] I tested changes to product code in product - [ ] I considered updates to the wiki Related work items: #952202
# Justification Implement Swagger routes. # Implementation Implement the route. Add tests. Add necessary mappings. # Testing Ran all tests. Validated in Swagger VM. # Checklist - [x] I tested changes to product code in product - [ ] I considered updates to the wiki Related work items: #952684
…nt HTTP API # Checklist - [ ] I tested changes to product code in product - [ ] I considered updates to the wiki
# Justification We need to implement the update policy route as specified in the .yml file. # Testing Added tests to cover the route and ran all the other tests. # Checklist - [x] I tested changes to product code in product - [ ] I considered updates to the wiki Related work items: #951070
# Justification Implement Swagger web service interface. # Implementation Implement the route. It does download or send to file ingestion. Added tests that check the 2 behaviors, but not the actual report content (it checks the columns and nb of rows). # Testing Ran all unit tests. # Checklist - [ ] I tested changes to product code in product - [ ] I considered updates to the wiki Related work items: #952695
# Justification Implement Swagger routes # Implementation Implements all cases for query-assets route. # Testing Ran all unit tests. Tested in product in VM. # Checklist - [X] I tested changes to product code in product - [ ] I considered updates to the wiki Related work items: #952679
# Justification The current web service returns an object that contains a list of packages. The current one returns directly the list of the packages. # Implementation Changed the return type in the yml file. # Testing Tested using SystemLink Web UI. Related work items: #947886
# Justification New openAPIGenerator nuget has support for static methods. Repository service needs this support for implementation of the download method. # Implementation Changed openAPIGenerator dependency. Removed duplicate definition of the download method in yml. Added "nirepo/v1" prefix concatenation for the packages and feeds uris. # Testing Generated classes with new nuget and tested the functionality of the program. Related work items: #947886
…nterface # Justification We have to use the Swagger gen code instead of the old http controller. # Implementation Update service class to instantiate the generated web service. Delete old classes that are not used any more. Delete some add external calibration test cases that were already covered at the facade level. Update all integration tests to work with new web service. Delete JsonConstructor and JsonProperty attributes from AppServices models. Increase timeout for a AMQP integration test for unknown reason (fails locally sometimes). # Testing Ran all unit and integration tests. Test in the product. # Checklist - [x] I tested changes to product code in product - [ ] I considered updates to the wiki Related work items: #956880
…match between x and y # Justification The need is to return data for the groups where we can return data, but return errors for the groups where there are errors. Also, possibly, return data AND errors for some groups depending on the error. For example: we truncated x or y because they didn’t match, so we’ll return you ‘something’ but it is lossy. # Implementation Add partial error return to query-data. Returning errors in error. # Testing added and modified existing tests. Tested in product. # Checklist - [x] I tested changes to product code in product - [ ] I considered updates to the wiki
# Justification Need to enable security for file ingestion # Implementation Turn on security in filegestion config file # Testing N/A # Checklist - [ x ] I tested changes to product code in product - [ x ] I considered updates to the wiki
…utilization response properties which can be empty. # Justification When we don't have utilization stored for some assets, we pad the response with utilization entries with 0 percent and no minion ID/utilization category. This was an issue as all the utilization properties were marked as 'required' and JSON serialization of the response would fail as they would be generated in C# with the Required=Always attribute. # Checklist - [X] I tested changes to product code in product - [ ] I considered updates to the wiki
…odels for the repository web service # Justification The workspace field is required for implementing organization modeling. # Implementation Add the workspace field in the description of the feed models. I added some more modifications to the yml file when I implemented the generated methods in order to make them behave accordingly to the old web service. I attach here a document with the differences if you want to review them as well: [diffs.zip](https://ni.visualstudio.com/94b22d7b-ad7b-4f5e-88f0-867910f91c94/_apis/git/repositories/8159353b-4403-4eed-9a32-0c2ee30abe3d/pullRequests/56954/attachments/diffs.zip) # Testing Successfully generated the new models.
…ilization HTTP APIs # Justification HTTP APIs for: Utilization Start Utilization End Utilization Heartbeat Of Course this will go in after changes related to codegen are in.
…ral utilization filter properties # Justification The example and the documentation of the POST /v1/query-asset-utilization route was wrong. It had : - "UtilizationCategory" as a possible filter query, but in fact it is just "Category". - "User" instead of "UserName" # Implementation Updated the swagger. # Testing Tested route with the correct property sent in filter. # Checklist - [x] I tested changes to product code in product - [ ] I considered updates to the wiki Related work items: #962514
# Justification The step data object is freeform, and the testmon UI looks for special keys in the step data object, but the OpenAPI doc does not describe what these keys are. # Implementation Use a more realistic example for step data, that utilizes some of the special values. # Testing Rendered docs. # Checklist ~~- [ ] I tested changes to product code in product~~ ~~- [ ] I considered updates to the wiki~~
We have to return the not found/unauthorized systems # Checklist - [x] I tested changes to product code in product - [ ] I considered updates to the wiki
# Justification
We need to expose the system utilization to http so that it can be used in Asset Utilization Notebook Report.
# Implementation
In the yml file I created QuerySystemUtilizationsRequest which represents the request model. Also in yml I created a model that represents the returned type of the result. Although query asset utilization and query system utilization return an object that contains the same properties and on amqp we could use the same model, on http I had to create a new one for query system utilization ('SystemUtilizationWithPercentageModel') because on the model returned by asset utilization, the assetIdentifier property is required and this property on query system utilization is always null.
Also in the yml file I entered the / v1 / query-system-utilization route using the created models.
# Testing
Unit test
I tested the route with Postman
# Checklist
- [x] I tested changes to product code in product
- [ ] I considered updates to the wiki
Query System Utilization to http
Related work items: #1001825
# Justification x-ni-privileges were used for the old web server privileges. Calls are being blocked if privileges are missing. For the new org modeling, we don't need them anymore. All calls can pass through and are being checked in the backend. # Implementation Removed x-ni-privileges from yml file
# Justification File service was still using NI Web Server privileges # Implementation - Deleted Privileges class - Added usage of PolicyEvaluator in all places where Privileges was used before - Removed x-ni-request-all-privileges - Updated OpenAPIGenerator nuget # Testing Manual testing Related work items: #1035085
# Justification The swagger doc missed to mention that workspace is an accepted query parameter # Implementation Added the missing element Related work items: #1047762
… without task name or without user name.
ServiceRegistry needs a way to tell the status of the service so we will make the Operations routes public instead of adding a new ping/up route # Checklist - [x] I tested changes to product code in product - [ ] I considered updates to the wiki
# Justification On the query system utilization response was returned an assetId that represents the id of a random asset in the system. This didn't bring any value so I removed the attribute from the message. # Implementation Removed assetId from SystemUtilizationWithPercentageResponse from yml file. # Testing I tested in the product I ran unit tests # Checklist - [x] I tested changes to product code in product - [ ] I considered updates to the wiki Related work items: #1047560
…ution # Justification The Service Registry service can use the Launcher service to be started and doesn't need the C# Elixir Wrapper anymore. # Implementation Removed the ServiceRegistry project from the Skyline Solution. I already added the Service Descriptor and Configuration JSON files in the installer # Testing - Verified that the Solution builds - Updated the Service Registry installer (in another changelist) and verified that it installs as expected and that it starts as expected # Checklist - [x] I tested changes to product code in product - [ ] I considered updates to the wiki
…ed for TDMS files # Justification Due to org modeling, the old web server privileges are not considered anymore. Therefore, the availability of operations should always return true. # Checklist - [x] I tested changes to product code in product - [x] I considered updates to the wiki Related work items: #1059635
# Justification #1046923 # Implementation Update swagger doc to use new auth keys # Testing Built and tested that 403 no longer occurs (cwaterma1/ - admin/admin) # Checklist - [X] I tested changes to product code in product - [X] I considered updates to the wiki Related work items: #1046923
…ror code # Justification #1055403 # Implementation Update swagger doc example error code # Testing None # Checklist - [X] I tested changes to product code in product - [X] I considered updates to the wiki Related work items: #1055403
# Justification Root routes must not require authentication. https://ni.visualstudio.com/DevCentral/_workitems/edit/1063728 # Implementation Removed "x-ni-auth:true" from the yml files. # Testing Tested locally # Checklist - [ ] I tested changes to product code in product - [ ] I considered updates to the wiki Related work items: #1063728
…service # Justification APM service has several routes that send a query result to the FileService. We must forward the ApiKey when sending the request to store the file. The user must specify the workspace where to store the file. # Implementation Update all routes (HTTP and AMQP) that sends data to FileIngestionService to have a new parameter that tells in which workspace to put the file. Forward the ApiKey. When the user does not have permissions to store the file, the error is forwarded back to the client (no change for this, already works). Enable again the FileIngestion integration tests and update then to have FileIngestionService permissions and use a valid workspace id. # Testing Tested local in the product and make all integration tests pass. # Checklist - [x] I tested changes to product code in product - [ ] I considered updates to the wiki Related work items: #1037900
# Justification Updated the open API document for the tag rule engine to reflect org modeling changes. # Implementation * Updated versions in TagRuleEngine's `V1Operations` * Added duplicate route * Added workspace field to rule * Added workspace param to query route * Added partial success responses to toggle and delete routes # Testing Validated changes in online swagger previewer - not sure if there's anything else I should check? # Checklist - [ ] I tested changes to product code in product - [ ] I considered updates to the wiki
…ocuments Pull in the latest changes from systemlink-openapi-documents Related work items: #81
…e in the query system # Justification We moved the AMQP query utilization APIs to HTTP and for some reason we forgot to add some properties that are used by the asset utilization notebook in the group by asset and group by system cases. # Implementation Added back the properties that are required by the asset utilization notebook, to not change the notebook to obtain those again. # Testing Tested locally. # Checklist - [ ] I tested changes to product code in product - [ ] I considered updates to the wiki Related work items: #1064880
BKnight760
approved these changes
Aug 4, 2020
cameronwaterman
pushed a commit
that referenced
this pull request
Apr 2, 2021
# Justification Our use of git subtree has always made it easy to pull changes from GitHub but difficult to push. Additionally, git subtree is why the repo has been configured to allow basic merges for PRs, even though we don't want to allow that for anything but a subtree PR. [git-subrepo](https://github.com/ingydotnet/git-subrepo) promises to be more reliable and not require any special merges, allowing us to not need ambiguous instructions or to allow basic merges for PRs. # Implementation - Created a helper script to download and set up git-subrepo on Windows. - Updated the README with instructions. - Used git-subrepo to [push our 20.1 changes to GitHub](#87). - Deleted and re-created the subtree using git-subrepo instead. # Testing Ran the `git subrepo branch` command to verify it'll work later in the release. # Checklist - [x] I tested changes to product code in product - [ ] ~~I considered updates to the wiki~~ - [ ] ~~If this PR affects user-visible strings, I added Jenna Jaco as a reviewer and attached screenshots of the strings in the product~~ Related work items: #1049591
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
16 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this Pull Request accomplish?
Pushes all changes made to the internal branch for the 2020 R2 release.
Why should this Pull Request be merged?
Makes available publicly what's being shipped in the product.
What testing has been done?
Did a diff between what's being pushed and what was shipped in 2020 R2.