Initialize Runtime Options with default values when absent in the config file#1678
Merged
Aniruddh25 merged 3 commits intorelease/0.8from Sep 1, 2023
Conversation
severussundar
requested review from
Aniruddh25,
JelteF,
Mathos1432,
aaronburtle,
aaronpowell,
abhishekkumams,
ayush3797,
gledis69,
mbhaskar,
milismsft,
ravishetye,
seantleonard and
tarazou9
as code owners
1 task
abhishekkumams
approved these changes
Sep 1, 2023
Contributor
abhishekkumams
left a comment
abhishekkumams
left a comment
There was a problem hiding this comment.
Thanks for investigating and adding the fix.
Aniruddh25
reviewed
Sep 1, 2023
Aniruddh25
reviewed
Sep 1, 2023
Aniruddh25
approved these changes
Sep 1, 2023
Collaborator
Aniruddh25
left a comment
Aniruddh25
left a comment
There was a problem hiding this comment.
Thanks for fixing this so soon.
Aniruddh25
deleted the
dev/shyamsundarj/handle-absent-global-rest-section-in-config-file
branch
severussundar
added a commit
that referenced
this pull request
Sep 14, 2023
…fig file (#1678) ## Why make this change? - Closes #1675 - For Cosmos DB NoSQL database type, DAB CLI v0.8.49 generates a REST property within the Runtime section of the config file. However v0.7.6 does not generate this property. So, when the config file generated using v0.7.6 is used to start the engine with v0.8.49, the absence of the REST property causes the engine to throw exceptions. ## What is this change? - With `v0.8.49`, these values are assumed to have non-null values because the CLI explicitly writes out all the fields to the config files. So, `null` checks are not performed extensively in all the places. - However, when a config file that is generated using `v0.7.6` is used, it could be possible that some of the fields are `null`. The absence of `null` checks leads to `NullReferenceExceptions` in this case. - To avoid this, all the `Runtime` options - `Rest`, `GraphQL` and `Host` are initialized with default values when they are `null` after deserialization of the config file. ## How was this tested? - [x] Existing Unit Tests and Integration Tests - [x] Manual Tests # Sample Requests - Database Type: Cosmos DB NoSQL - Config file (Runtime section): ```json "runtime": { "graphql": { "enabled": true, "path": "/graphql", "allow-introspection": true }, "host": { "cors": { "origins": [ "http://localhost:5000" ], "allow-credentials": false }, "authentication": { "provider": "StaticWebApps" }, "mode": "development" } } ``` - Engine starts successfully:  - GraphQL Requests work successfully:   Note: This PR is directly targeted towards `release/0.8` branch (and the branch to make these changes was snapped off of `release/0.8`) because the current `main` has additional changes that are related to `0.9` and porting it over to `release/0.8` branch might not be possible. --------- Co-authored-by: Aniruddh Munde <[email protected]>
2 tasks
ayush3797
pushed a commit
that referenced
this pull request
Sep 14, 2023
…or 0.8.* version) to the main branch (#1712) Ports the patch fix PR #1678 which was directly merged into `release/0.8` to the `main` branch so that it is avaiable for future versions ## Original PR description: ## Why make this change? - Closes #1675 - For Cosmos DB NoSQL database type, DAB CLI v0.8.49 generates a REST property within the Runtime section of the config file. However v0.7.6 does not generate this property. So, when the config file generated using v0.7.6 is used to start the engine with v0.8.49, the absence of the REST property causes the engine to throw exceptions. ## What is this change? - With `v0.8.49`, these values are assumed to have non-null values because the CLI explicitly writes out all the fields to the config files. So, `null` checks are not performed extensively in all the places. - However, when a config file that is generated using `v0.7.6` is used, it could be possible that some of the fields are `null`. The absence of `null` checks leads to `NullReferenceExceptions` in this case. - To avoid this, all the `Runtime` options - `Rest`, `GraphQL` and `Host` are initialized with default values when they are `null` after deserialization of the config file. ## How was this tested? - [x] Existing Unit Tests and Integration Tests - [x] Manual Tests # Sample Requests - Database Type: Cosmos DB NoSQL - Config file (Runtime section): ```json "runtime": { "graphql": { "enabled": true, "path": "/graphql", "allow-introspection": true }, "host": { "cors": { "origins": [ "http://localhost:5000" ], "allow-credentials": false }, "authentication": { "provider": "StaticWebApps" }, "mode": "development" } } ``` - Engine starts successfully:  - GraphQL Requests work successfully:   Note: This PR is directly targeted towards `release/0.8` branch (and the branch to make these changes was snapped off of `release/0.8`) because the current `main` has additional changes that are related to `0.9` and porting it over to `release/0.8` branch might not be possible. Co-authored-by: Aniruddh Munde <[email protected]>
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
3 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.
Why make this change?
What is this change?
v0.8.49, these values are assumed to have non-null values because the CLI explicitly writes out all the fields to the config files. So,nullchecks are not performed extensively in all the places.v0.7.6is used, it could be possible that some of the fields arenull. The absence ofnullchecks leads toNullReferenceExceptionsin this case.Runtimeoptions -Rest,GraphQLandHostare initialized with default values when they arenullafter deserialization of the config file.How was this tested?
Sample Requests
Engine starts successfully:
GraphQL Requests work successfully:
Note: This PR is directly targeted towards
release/0.8branch (and the branch to make these changes was snapped off ofrelease/0.8) because the currentmainhas additional changes that are related to0.9and porting it over torelease/0.8branch might not be possible.