Skip to content

Conversation

seantleonard
Copy link
Contributor

@seantleonard seantleonard commented Mar 8, 2024

Why make this change?

What is this change?

  • Updates the ProductInfo functions to return either a version number with major.minor.patch or major.minor.patch+<commit_hash>. The only time the version is returned with the commit hash is when the CLI is used with either --version, --help or an unrecognized verb/option. All other instances of version printing to console in engine, cli, and tests should NOT include the commit hash.
{
    "status": "Healthy",
    "version": "0.12.0",
    "appName": "dab_oss_0.12.0"
}
  • Added the classes

    1. DabHealthCheck : IHealthCheck
    2. HealthReportResponseWriter -> uses Health Check metadata to fill health check endpoint response contents (JSON String). Because the writer writes raw json and uses values that stay constant through the lifetime of the dab process, the created json is cached after first creation and returned whenever the health endpoint is hit.
  • Updates Directory.Build.props with <Version>0.12</Version> to avoid having to hardcode version numbers in all .csproj files like in add current version number #1995 , and this hardcoded version number (just with Major.Minor) enables the Open Source version of DAB (when forked, built locally, or tested locally without the benefit of having CI/CD injected versioning.)

    • Updates the BuildPipeline.yml to include a powershell script prior to other steps which parses the major/minor version specified in the file Directory.Build.props and uses those values instead of pipeline yml hardcoded values. While we still are hardcoding major/minor, we are doing so in code that will allow the scenarios mentioned (local builds/tests) to have versioning aligning with the current dab development milestone (sans the patch value tracked by ci/cd). This will prevent local builds reporting some default catch all version like 1.0.0
    • This means that the hardcoded variables $major and $minor are removed from the pipeline. The $patch variable assignment still has the counter() function determining its value and *as long as the powershell script parsing Directory.Build.props gets executed prior to other steps, then there will be a value set for major/minor for the counter() function to use when the variable is queried. In other words, the variables "major" and "minor" need not be set directly in "variables". This is tested and demonstrated in the pipeline.
      Reproducible PowerShell script you can run locally in powershell ise:
[xml]$directoryBuildProps = Get-Content -Path "C:\dabdev\src\Directory.Build.props"
# Get Version from Directory.Build.props
$version = $directoryBuildProps.Project.PropertyGroup.Version[0]
# Get Major , Minor, Patch from Version
$major = $version.Split([char]'.')[0]
$minor = $version.Split('.')[1]
# store $major and $minor powershell variables into azure dev ops pipeline variables
Write-Host "##vso[task.setvariable variable=major]$major"
Write-Host "##vso[task.setvariable variable=minor]$minor"
  • OpenAPI document now references the DAB version instead of the string "PREVIEW" which is also reflected in SWAGGER UI
    image

How was this tested?

  • Integration Tests
    • CLI tests checking version number
    • [IN PROGRESS], actual test for health check classes
    • [DONE], mitigates: add current version number #1995 (comment) , where Directory.Build.props has precedence for version number and pipeline uses that value and supplements it with the patch version.

Sample Request(s)

GET https://localhost:5001

@seantleonard seantleonard linked an issue Mar 8, 2024 that may be closed by this pull request
@abhishekkumams
Copy link
Contributor

abhishekkumams commented Mar 28, 2024

can we add a test where we see the app_name as dab_hosted_1.0.0 for hosted case and for oss dab_oss_1.0.0 ?

Copy link
Contributor

@aaronburtle aaronburtle left a comment

Choose a reason for hiding this comment

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

It looks good. Few comments about formatting, and some clarifying questions.

…ms. application name in connection string test logic updated to be more readable with fewer params and better named params. Health endpoint tests updated to breaking long lines into more readable 'if' blocks.
@seantleonard seantleonard dismissed Aniruddh25’s stale review March 29, 2024 20:46

new changes since last review, so existing review stale.

@seantleonard
Copy link
Contributor Author

/azp run

@Azure Azure deleted a comment from azure-pipelines bot Apr 1, 2024
@seantleonard
Copy link
Contributor Author

/azp run

@seantleonard seantleonard enabled auto-merge (squash) April 1, 2024 18:52
@seantleonard seantleonard merged commit 7f7c673 into main Apr 1, 2024
@seantleonard seantleonard deleted the dev/sean/1971_version_health_endpt branch April 1, 2024 19:15
@seantleonard seantleonard mentioned this pull request Apr 1, 2024
2 tasks
@yorek
Copy link
Contributor

yorek commented Apr 1, 2024

Love it!!!!

@JerryNixon
Copy link
Contributor

This is awesome!

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.

Improved HealthEndpoint (Return deployed version at root path)
6 participants