Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add current version number #1995

Closed
wants to merge 4 commits into from
Closed

add current version number #1995

wants to merge 4 commits into from

Conversation

yorek
Copy link
Contributor

@yorek yorek commented Jan 30, 2024

Why make this change?

Add a default version that is aligned with what being built to be able to more easily identify the correct version of a dab application connected to MSSQL (or another supported database)

What is this change?

The change adds current version information to the project files.

How was this tested?

  • Integration Tests
  • Unit Tests

Notes

With this change, querying MSSQL to find the connected DAB instances will return also correct version information:

select session_id, host_name, program_name, client_interface_name 
from sys.dm_exec_sessions where is_user_process = 1 and program_name like '%dab%'

result:

image

@abhishekkumams
Copy link
Contributor

adding the version in project file will override the version that is generated automatically in build. Therefore, it makes things more manual. we need to find some better way.

@Aniruddh25
Copy link
Contributor

adding the version in project file will override the version that is generated automatically in build. Therefore, it makes things more manual. we need to find some better way.

build automatically generates only the patch number. Major/Minor versions are hard coded in the build pipeline, and since FileTransform will only be useful for automated builds, I guess the only way to have these versions in local cloned builds is adding them in the csproj files as well.

@Aniruddh25
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

Copy link
Contributor

@Aniruddh25 Aniruddh25 left a comment

Choose a reason for hiding this comment

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

There seems to be some unit tests in the CLI project that are failing which still are expecting 1.0.0 version.

@abhishekkumams
Copy link
Contributor

abhishekkumams commented Feb 1, 2024

adding the version in project file will override the version that is generated automatically in build. Therefore, it makes things more manual. we need to find some better way.

build automatically generates only the patch number. Major/Minor versions are hard coded in the build pipeline, and since FileTransform will only be useful for automated builds, I guess the only way to have these versions in local cloned builds is adding them in the csproj files as well.

We can't merge this PR, before testing the FileTransform task, because the version hardcoded in csproj file will take precedence over the nuget pack command that we have in build pipeline. this will create an issue as it won't use the generated version.

@seantleonard
Copy link
Contributor

adding the version in project file will override the version that is generated automatically in build. Therefore, it makes things more manual. we need to find some better way.

build automatically generates only the patch number. Major/Minor versions are hard coded in the build pipeline, and since FileTransform will only be useful for automated builds, I guess the only way to have these versions in local cloned builds is adding them in the csproj files as well.

We can't merge this PR, before testing the FileTransform task, because the version hardcoded in csproj file will take precedence over the nuget pack command that we have in build pipeline. this will create an issue as it won't use the generated version.

What specific steps are needed beyond ensuring existing pipelines pass? What logic does nuget pack command that we have in build pipeline. have and can we update that to accommodate this change?

seantleonard added a commit that referenced this pull request Apr 1, 2024
## Why make this change?

- Closes #1971, where the health point only returned a plain text
`Healthy`. There was no additional information that would be useful such
as DAB version.

## 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.

```json
{
    "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
#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:
```powershell
[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](https://github.com/Azure/data-api-builder/assets/6414189/84c21203-653f-4b18-ae7b-5d5cf2ea97ab)

## How was this tested?

- [X] Integration Tests
    - CLI tests checking version number
    - [IN PROGRESS], actual test for health check classes
- [DONE], mitigates:
#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)

```https
GET https://localhost:5001
```
@seantleonard
Copy link
Contributor

closing because this functionality was fixed in #2086

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants