Sync OpenAPI documents from AzDO#90
Merged
cameronwaterman merged 29 commits intomasterfrom Apr 2, 2021
Merged
Conversation
Currently when trying to generate code using editor.swagger.io from this file it returns the following error:
```
Semantic error at paths./v1/files.parameters.0.name
Path parameter "pathToFile" must have the corresponding {pathToFile} segment in the "/v1/files" path
Jump to line 942
```
- Adding {pathToFile} to the route string
- Tested that I can generate code from the swagger document.
Fix syntax error in nirepo swagger file
# 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
# Justification The execution record includes all the notebook data (parameters, path, etc) except the metadata blob. The V2 notebook output format only stores the version number in the metadata blob, and not in the actual notebook output. Combined with the above, this means that execution records don't actually say which version the results are in - the output blob is present in the execution result without an actual version number. We also can't simply start adding the version number to notebook output either, since there now exist notebooks in the V2 format which do not record a version number. Therefore, the only non-breaking way to ensure the version number appears in the execution record is to add the metadata blob. # Implementation Updates the swagger doc to include the metadata blob. Corresponding implementation changelist is WIP on the perforce side. # Testing Rendered this documentation in the Swagger editor. # Checklist ~~- [ ] I tested changes to product code in product~~ ~~- [ ] I considered updates to the wiki~~ - [X] If this PR affects user-visible strings, I added Jenna Jaco as a reviewer and attached screenshots of the strings in the product
# Justification Our HTTP API (swagger) marks the version parameters as optional. However we require this fields in lower layers of implementation. We already validate (and throw exceptions) when the version is missing so this change makes sure the API is in sync with the implementation. __Note: we could also just make the version optional all the way, but that is a larger change and I don't think there's a business case for it__ # Implementation - Change API - Make sure we validate parameters in upper layers too - Make sure valid error messages are returned # Testing - Some manual testing - ? Extend existing unit tests ? # Checklist - [ ] I tested changes to product code in product - [x] I considered updates to the wiki - [x] 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: #1104541
# Justification The python module already supports this but defaults to `False` (which is the expected use-case): ```python def installed(name, version, reinstall=False, **kwargs): ``` I'm just adding it to the HTTP API too, so that users have the option to erase targets. Still defaults to false, since this is more like an advanced use-case. __I think it's ok if this new flag is not reflected in the UI (that can be another change). But at least we officially support this in the raw editor and using the HTTP APIs__ # Implementation Add optional flag to the model # Testing - Updated existing tests (default use-case with `False`) - Added new test for `True` flag # Checklist - [ ] 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: #1108603
…rors more accurately # Justification The initial issue was that we first delete the file from the database and then try to delete the file. It's better to do this the other way round because it's less likely that we fail to delete the database entry after we deleted the file. While investigating it turned out that we don't handle errors very smoothly if multiple files couldn't be deleted. # Implementation Refactored the strategy how to delete multiple files and handle a variety of common outcomes. # Testing Manual testing Related work items: #1094146
…er doc # Justification The API didn't tell how to rename a file. # Implementation Changed the description for the properties map which can be used to rename the file.  Related work items: #1079968
# Justification
We need to return CustomCalibrationInterval on v1/assets, v1/query-assets, v1/assets/{assetId}, v1/update-assets Http Responses
# Implementation
Added property on yml file and Models
# Testing
Manual testing
Checked in Swagger Editor
# 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: #1134842
# Justification
Based on the requirements of the referenced bug, I've updated the Tag API Swagger Documentation to account for 2 new routes:
- / -> Root Endpoint
- /v2 -> Root Endpoint Version 2
- /v2/tags/{workspace}/{path}/values/current
# Implementation
Added the new documentation to the Swagger document
# Testing
Verified that the Swagger documentation does not contain errors and that is rendered as expected
# 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: #1037753
…e asset route # Justification #1139752 # Implementation Added the workspace property to the API and appropriate mappers. # Testing Manual and unit tests. # 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: #1139760
… policy action on FileIngestion query route # Justification For the FileMoving service to use HTTP for RPC communication, the file path on the server needed to be changed from the AMQP message to accessible over HTTP. # Implementation Implemented a new file-paths:Read action in the FileIngestion service. This is used during the file query and returns file paths only if the API key used in the request has the privilege. # Testing Installed the resulting FileIngestion and FileMoving binaries on my server and verified that the FileIngestion service returns the file path when the privilege is present and returns a null path element when the privilege is not present. # 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~~
…asset # Justification #1139761 # Implementation Added the workspace mapper to the asset update mapper. Changed the workspace mapper such that it checks for workspace collision when the location is also specified. Added the workspace field to the asset update class. # Testing Unit and manual testing. # 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: #1139761
…er doc # Justification #1092413 # Implementation Add "oidc-claim" mapping type to user services swagger doc # Testing Verified no syntax errors by pasting into edditor.swagger.io # Checklist - [X] I tested changes to product code in product - [X] I considered updates to the wiki - [X] 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: #1092413
…ystemLink states - backend part # Justification Support software deploy on ubuntu clients through SystemLink states # Implementation Added "Any" for the distribution. # Testing Used manual test. # Checklist - [ ] 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
…ssets results. # Justification #1165330 In order to be able to check if an asset is added manually or automatically in the angular app, we need to expose the "discoveryType" property. # Implementation Added the 'discoveryType' property to the API models. # Testing Two new facade tests. Ran all existing tests. # 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: #1165330
…nd AssetCreateModel in YML Did not move the existing properties from AssetModel to avoid changing the order of parameters for that model. We ran into some issues when creating model without named parameters in Python on this. All new parameters now should go at the end.
# Justification ActivateSystems do not response 204 No Content. In the success case, it return 200 PartialSuccess. # Implementation Deleted the response 204 No Content for /v1/activate-systems in Swagger. # Checklist - [ ] 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
…ailability report. # Justification We need to find o way of making the asset reports execute faster. In this case, the Connection History report will benefit from using a route that returns the data in csv format. # Implementation A new route was added to the asset service that returns availability data in the csv format. Pull Request 116981: Add a csv route for asset availability. https://ni.visualstudio.com/DevCentral/_git/Skyline/pullrequest/116981 # Testing Check the PR linked above, it contains the required tests for the route and also some performance benchmarks. # Checklist - [x] I tested changes to product code in product - [x] I considered updates to the wiki - [x] If this PR affects user-visible strings, I added Jenna Jaco as a reviewer and attached screenshots of the strings in the product Asset Service - Asset route for the query assets availability report.
… being utilized # Justification Currently, there is no fixed API that tells the user what assets are currently being marked as being utilized by an utilization session. The user would need to use the "query-asset-utilization-history" route with some specific utilization filters to get the active utilization entries. After that, the user would have to use the "query-assets" route with some specific asset filters (based on the response from the "query-asset-utilization-history" route) to now get information about the assets currently being utilized. The "Assets" grid needs to display asset summary information in terms of how many assets are currently being utilized. The UI will call into this new route to only retrieve the assets currently being marked for utilization # Implementation Add a new "query-utilized-assets" route that allows users to query for assets for which an utilization session is still in progress The route has support for: - asset filters -> Query for specific asset properties - utilization filters -> Query for specific utilization properties Note: The associated utilization sessions for assets to still be considered as "in use" have the following requirements: - EndTimestamp should be NULL - If there is NO HeartbeatTimestamp => StarTimestamp should be greater than DateTime.Now - 10 minutes - If there is a HeartbeatTimestamp => HeartbeatTimestamp should be greater than DateTime.Now - 10 minutes -> Basically the status of the utilization session should have been updated at least once in the last 10 minutes when the request was being made) Added a stub route to the "AssetPerformanceManagement" HTTP Service because of Swagger # Testing - Added facade level unit tests - Added integration tests # 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
…t in use assets # Justification For the summary panel in the UI, we need to extend the assets-summary route so that we will have less requests for data to be populated in the panel. # Implementation I have added two new properties to the `AssetsCount` model to extend it to support `inUse` and `notInUse`. I have also changed the way we compute the values in GetAssetsCount, so that we won't need to many queries on the db, and work with the data in memory. # Testing I have added an integration test and also ran all the tests in the AssetSummaryTest class. # 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
# Justification For the previous releases setting no take to the query-assets route would get all the assets. Right now, the take defaults to 1000. The problem was that 1000 was also the maximum allowed when creating the request via the generated API. This was making it impossible for the clients to query more than 1000 assets. This means that the notebooks were only taking into consideration the first 1000 assets. (The 20.6 released version was doing query-assets with no take and was getting all assets, so this is not a released issue) # Implementation Removed the maximum value from the take specification in niapm.yml. # Checklist - [ ] 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 The query-assets take should not have a max value.
…eating assets # Justification As the service will handle creation of assets via the Create-Assets route, the request model for it needs to support specifying the discovery type # Implementation Added the discovery type enum to the request and mapper. # Testing Added an integration test. # 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 Added support for specifying discovery type when creating assets Related work items: #1324165
# Justification #1224376 This closes out the remaining task in this story - adding support for current time timestamp tag rules. # Implementation ####General strategy: - Support timespans as a valid set point for timestamp tag conditions - When evaluating the condition, compare the tag's value to the server's time +- the setpoint. (e.g. 02:00:00 means two hours past the current time) - Since the passage of time can cause these rules to become active, we need to evaluate them on a regular interval ####Details: In ConditionRecord.cs, setpoints are converted to a TypedValue that matches the tag's type. The assumption that the types will be the same doesn't hold for timespan setpoints, so I short-circuited the conversion in SetTypedValueFields. Then when evaluating timestamp rules, we can get the setpoint from the untyped list when it's a timespan. It's a little gross, but creating a new interface for typing setpoints would be a ton of overhead, and this is probably the only case we'll ever have where the set point type doesn't match the tag type. When parsing timespans, I'm manually passing in a culture. Otherwise, the input format could change depending on where the server is installed. The TagRuleEngine now uses the Timer service to schedule callbacks to evaluate relative time rules. The interval length is stored in the config file for the service. For this task, we want the callback to be a work subscriber, but the timer service currently only supports instance subscribers. Fixing this was a pretty simple change - I added a boolean to the RegisterCallback message that tells the timer service to route the response message back to the subscriber message's origin (which is just the service's name). # Testing - Built and tested the changes on a fresh 21.0 SystemLink install - Added a new test to the ATS # Checklist - [x] I tested changes to product code in product - [x] I considered updates to the wiki - [x] 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: #1224376
# Justification #1224749 # Implementation Added a new tag data type of `TAG_TIME`. For rules with this type, we use a tag's last updated timestamp as the value to compare against. Everything else works the same as with `DATE_TIME` tags. To keep the types in ConditionRecord the same, I manually instantiate a TypedValue and set its value to the tag's timestamp before continuing with the evaluation. This is a little gross because TypedValue only has an empty constructor and is probably not intended to be manually created, so I'm open to suggestions. Normally, rules will only match against tag's with the same type as the rule, but `TAG_TIME` rules will match tags of any type. The way this all works is unfortunately opaque - the TagRuleEngine service queries for all tags that match a rule's search path without taking tag types into consideration. Instead, the `evaluate` method in ConditionRecord is called for every matching tag, but will return an error (which never gets bubbled up anywhere) if the rule's setpoints don't match the incoming value's type. For `TAG_TIME` rules, every tag update will always include a timestamp, so that check never fails. This PR also includes a tiny bug fix for #1275650 to not evaluate disabled rules. There was already a Debug.Assert for this case, so it was easy to return a 'none' evaluation result instead. # Testing - Built & tested on a fresh install of 21.0 SystemLink - Added a test to the ATS for last updated time rules # Checklist - [x] I tested changes to product code in product - [x] I considered updates to the wiki - [x] If this PR affects user-visible strings, I added Jenna Jaco as a reviewer and attached screenshots of the strings in the product
# Justification As per discussion with PO, the default DiscoveryType enum value should be MANUAL.
…trieve the current user's workspaces and roles #943683 Part of this feature includes a new UI that displays the workspaces and roles that the current user is associated with. The spec included in this PR discusses options for retrieving data for this view. Related work items: #1356161
# Justification #1338001 The definition in `nitdmreader.yml` didn't match the actual implementation and response. This caused the generated Python client to deserialize something it doesn't expect. # Implementation Adjust `FileInformation` to define only a single file information and add a `FileInformationList` definition which is a list of `FileInformation`. Related work items: #1338001
subrepo: subdir: "systemlink-openapi-documents/subtree" merged: "36f2c3d417" upstream: origin: "https://github.com/ni/systemlink-OpenAPI-documents.git" branch: "master" commit: "d791692168" git-subrepo: version: "0.4.3" origin: "???" commit: "???"
maxgax
approved these changes
Apr 2, 2021
Contributor
|
Looks good to me |
vladbaja
pushed a commit
that referenced
this pull request
Jul 1, 2021
Sync OpenAPI documents from AzDO
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
13 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?
Sync OpenAPI documents from AzDO
Why should this Pull Request be merged?
This repo is out-of-date
What testing has been done?
N/A