Skip to content

Conversation

@severussundar
Copy link
Contributor

@severussundar severussundar commented Dec 12, 2022

Why make this change?

  • Closes Change parameter generateConfigFiles to accept a database type #1015
  • At the moment, in all the integration testing pipelines, the command dotnet build -p:generateConfigFiles=true is used to generate the config files.
  • This command generates the config files for all the database types (MsSql, MySql, PostgreSql and Cosmos).
  • This is unnecessary as only one of the config files gets utilized during a given pipeline run i.e. in the MsSql pipeline, it would be sufficient to generate only the config file for MsSql database type. Likewise, for other pipelines. Additionally, generating the config files for all database types leads to longer execution times for the pipeline runs.

What is this change?

  • generateConfigFiles property is re-named to generateConfigFileForDbType.
  • generateConfigFileForDbType property accepts the database type (instead of true/false) and generates the config file for only that particular database.
  • All the integration testing pipelines are updated to specify the database type in the build step
  • The docs are updated with the necessary instructions

How was this tested?

  • Existing Unit tests and Integration tests

Sample Request(s)

  • N/A

@severussundar severussundar changed the title Generate only the specified Config File along with build Remove generation of multiple config files along with build Dec 12, 2022
@severussundar severussundar changed the title Remove generation of multiple config files along with build Skip generation of multiple config files along with build Dec 12, 2022
@severussundar severussundar marked this pull request as ready for review December 12, 2022 10:17
Copy link
Contributor

@abhishekkumams abhishekkumams left a comment

Choose a reason for hiding this comment

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

LGTM, just one nit. Thanks for making the build faster.

Copy link
Contributor

@seantleonard seantleonard left a comment

Choose a reason for hiding this comment

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

Thank you for updating this! Will definitely speed up the CI/CD runs, because generating config files for all databases was VERY time consuming. Sometimes took longer than the actual tests.

@severussundar severussundar merged commit c359ae3 into main Dec 14, 2022
@severussundar severussundar deleted the dev/shyamsundarj/modify-generateConfigFiles-parameter branch December 14, 2022 05:49
abhishekkumams pushed a commit that referenced this pull request Dec 16, 2022
* accepting database type instead of bool

* using database type in pipelines

* changing leftover param to database type

* updating docs

* renaming param to generateConfigFile

* renaming param to generateConfigFileForDbType
abhishekkumams pushed a commit that referenced this pull request Dec 16, 2022
* accepting database type instead of bool

* using database type in pipelines

* changing leftover param to database type

* updating docs

* renaming param to generateConfigFile

* renaming param to generateConfigFileForDbType
abhishekkumams added a commit that referenced this pull request Dec 19, 2022
* Skip generation of multiple config files along with build (#1029)

* accepting database type instead of bool

* using database type in pipelines

* changing leftover param to database type

* updating docs

* renaming param to generateConfigFile

* renaming param to generateConfigFileForDbType

* Adding support for nullable/varchar(*) result columns for stored-procedure (#1038)

* adding support for nullable result columns

* fix formatting

* fixing nits

* fixing test

* Fix broken links (#1046)

* Removing references to Cosmos Database in CLI/Engine/Pipelines/Config file names/Build commands. (#1045)

* Removing references to Cosmos DatabaseType in CLI/Engine code

* updating cosmos commands

* updating scripts to use cosmosdb_nosql

* updating pipeline files

* updating cosmos -> cosmosdb_nosql

* Renaming cosmos config file

* fix pipeline issue

* fix pipeline issue

* fix pipeline issue

* updating launch settings

* debugging error

* debugging error

* debugging error

* debugging error

* Updating schema -> graphql-schema for cosmosdb_nosql

* Reverting graphql-schema -> schema

* Replacing references to cosmos with cosmosdb_nosql

Co-authored-by: Ayush Agarwal <agarwalayush@microsoft.com>

* Cli check for stored procedure Operations (#1041)

* validation for stored-procedure permission in CLI

* fix formatting

* fixing tests

* fix formatting

* fix summary

* fixing nits

* fix formatting

* fix formatting

* addressing leftover comments from PR-1045 (#1052)

* addressing leftover comments from PR-1045

* updating remaining references to cosmos in readme

* updating cosmos getting started doc

Co-authored-by: Ayush Agarwal <agarwalayush@microsoft.com>

Co-authored-by: Shyam Sundar J <shyamsundarj@microsoft.com>
Co-authored-by: Ayush Agarwal <34566234+ayush3797@users.noreply.github.com>
Co-authored-by: Ayush Agarwal <agarwalayush@microsoft.com>
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.

Change parameter generateConfigFiles to accept a database type

5 participants