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

Handling Proper Defaulting of BuildTargetingString within nested templates #21426

Merged
merged 22 commits into from
Oct 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
dd1d8a2
adding coalesc everywhere
scbedd Oct 26, 2021
c0ac841
Revert "adding coalesc everywhere"
scbedd Oct 26, 2021
e7f1797
change the diff, allow it to set at the CI level
scbedd Oct 26, 2021
5d13c79
trying to add the coalesce everywhere we use it
scbedd Oct 27, 2021
ace8624
removing changes to core ci.yml
scbedd Oct 27, 2021
1de7be4
don't trigger core changes anymore. update how we call into this thing
scbedd Oct 27, 2021
4c27f5c
update to runtime syntax
scbedd Oct 27, 2021
a61dc4f
attempt usage
scbedd Oct 27, 2021
20d39af
update how we handle this thing
scbedd Oct 27, 2021
b83e3e7
adjust to variable access syntax instead of macro syntax
scbedd Oct 27, 2021
3e98ee1
update line
scbedd Oct 27, 2021
4f23403
only run when necessary
scbedd Oct 27, 2021
eff1f56
adjust targeting string. remove erroneous print
scbedd Oct 27, 2021
eb4f2a6
Update eng/pipelines/templates/steps/targeting-string-resolve.yml
scbedd Oct 27, 2021
3832b2d
update to resolve the targeting string wherever it is necessary to do so
scbedd Oct 28, 2021
7fa88c0
re-enable commented code. remove duplicate step in build-artifacts.yml
scbedd Oct 28, 2021
487d66a
first wave of changes that remove BuildTargetingStringParameter. only…
scbedd Oct 29, 2021
10caa01
fix spacing
scbedd Oct 29, 2021
25cf151
resolve conflict in livetest config
scbedd Oct 29, 2021
efa6278
add the comment explaining where TargetingString comes from into set-…
scbedd Oct 29, 2021
7ebe679
link verification called out an issue
scbedd Oct 29, 2021
c58935e
removing detail that was confusing in eng_sys_checks
scbedd Oct 29, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions eng/pipelines/templates/jobs/ci.tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,10 @@ jobs:
value: ${{ parameters.InjectedPackages }}

steps:
- template: /eng/pipelines/templates/steps/targeting-string-resolve.yml
parameters:
BuildTargetingString: ${{ parameters.BuildTargetingString }}

- template: /eng/common/pipelines/templates/steps/verify-agent-os.yml
parameters:
AgentImage: $(OSVmImage)
Expand Down Expand Up @@ -99,7 +103,6 @@ jobs:
OSVmImage: $(OSVmImage)
CoverageArg: $(CoverageArg)
PythonVersion: $(PythonVersion)
BuildTargetingString: ${{ parameters.BuildTargetingString }}
ToxTestEnv: $(toxenv)
ToxEnvParallel: ${{ parameters.ToxEnvParallel }}
InjectedPackages: $(InjectedPackages)
Expand All @@ -112,7 +115,6 @@ jobs:
- template: ../steps/set-dev-build.yml
parameters:
ServiceDirectory: ${{ parameters.ServiceDirectory }}
BuildTargetingString: ${{ parameters.BuildTargetingString }}

- ${{ each step in parameters.BeforeTestSteps }}:
- ${{ step }}
Expand Down
15 changes: 12 additions & 3 deletions eng/pipelines/templates/jobs/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,13 @@ jobs:
vmImage: MMSUbuntu20.04

steps:
- template: /eng/pipelines/templates/steps/targeting-string-resolve.yml
Copy link
Member Author

Choose a reason for hiding this comment

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

Just at the top of each job.

parameters:
BuildTargetingString: ${{ parameters.BuildTargetingString }}

- template: ../steps/build-artifacts.yml
parameters:
ServiceDirectory: ${{ parameters.ServiceDirectory }}
BuildTargetingString: ${{ parameters.BuildTargetingString }}
BeforePublishSteps: ${{ parameters.BeforePublishSteps }}
BuildDocs: ${{ parameters.BuildDocs }}
TestPipeline: ${{ parameters.TestPipeline }}
Expand All @@ -98,6 +101,10 @@ jobs:
vmImage: MMSUbuntu20.04

steps:
- template: /eng/pipelines/templates/steps/targeting-string-resolve.yml
parameters:
BuildTargetingString: ${{ parameters.BuildTargetingString }}

- template: /eng/common/pipelines/templates/steps/check-spelling.yml

- template: /eng/common/pipelines/templates/steps/verify-links.yml
Expand All @@ -112,7 +119,6 @@ jobs:
- template: ../steps/analyze.yml
parameters:
ServiceDirectory: ${{ parameters.ServiceDirectory }}
BuildTargetingString: ${{ parameters.BuildTargetingString }}
TestMarkArgument: ${{ parameters.TestMarkArgument }}
AdditionalTestArgs: '--wheel_dir="$(Build.ArtifactStagingDirectory)"'
TestPipeline: ${{ parameters.TestPipeline }}
Expand Down Expand Up @@ -180,7 +186,10 @@ jobs:
vmImage: MMSUbuntu20.04

steps:
- template: /eng/pipelines/templates/steps/targeting-string-resolve.yml
parameters:
BuildTargetingString: ${{ parameters.BuildTargetingString }}

- template: ../steps/test_regression.yml
parameters:
ServiceDirectory: ${{ parameters.ServiceDirectory }}
BuildTargetingString: ${{ parameters.BuildTargetingString }}
7 changes: 4 additions & 3 deletions eng/pipelines/templates/jobs/live.tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,6 @@ jobs:
value: true
- name: ArmTemplateParameters
value: '@{}'
- name: BuildTargetingStringValue
value: $[ coalesce(variables['BuildTargetingString'], '${{ parameters.BuildTargetingString }}') ]

timeoutInMinutes: ${{ parameters.TestTimeoutInMinutes }}
continueOnError: false
Expand All @@ -87,6 +85,10 @@ jobs:
parameters:
AgentImage: $(OSVmImage)

- template: /eng/pipelines/templates/steps/targeting-string-resolve.yml
parameters:
BuildTargetingString: ${{ parameters.BuildTargetingString }}

- template: /eng/common/TestResources/build-test-resource-config.yml
parameters:
SubscriptionConfiguration: ${{ parameters.CloudConfig.SubscriptionConfiguration }}
Expand All @@ -101,7 +103,6 @@ jobs:

- template: ../steps/build-test.yml
parameters:
BuildTargetingString: $(BuildTargetingStringValue)
ServiceDirectory: ${{ parameters.ServiceDirectory }}
CloudName: ${{ parameters.CloudConfig.Cloud }}
CoverageArg: $(CoverageArg)
Expand Down
5 changes: 1 addition & 4 deletions eng/pipelines/templates/stages/archetype-sdk-client.yml
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,6 @@ parameters:

stages:
- stage: Build
variables:
- name: BuildTargetingStringValue
value: $[ coalesce(variables['BuildTargetingString'], '${{ parameters.BuildTargetingString }}') ]
jobs:
- template: /eng/pipelines/templates/jobs/ci.yml
parameters:
Expand All @@ -87,7 +84,7 @@ stages:
AfterTestSteps: ${{ parameters.AfterTestSteps }}
BeforePublishSteps: ${{ parameters.BeforePublishSteps }}
TestMarkArgument: ${{ parameters.TestMarkArgument }}
BuildTargetingString: $(BuildTargetingStringValue)
BuildTargetingString: '${{ parameters.BuildTargetingString }}'
TestTimeoutInMinutes: ${{ parameters.TestTimeoutInMinutes }}
ToxEnvParallel: ${{ parameters.ToxEnvParallel }}
InjectedPackages: ${{ parameters.InjectedPackages }}
Expand Down
27 changes: 8 additions & 19 deletions eng/pipelines/templates/steps/analyze.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
parameters:
BuildTargetingString: 'azure-*'
ServiceDirectory: ''
TestMarkArgument: ''
AdditionalTestArgs: ''
Expand All @@ -8,6 +7,12 @@ parameters:
VerifyAutorest: false
ValidateFormatting: false

# The variable TargetingString is set by template `eng/pipelines/templates/steps/targeting-string-resolve.yml`. This template is invoked from yml files:
# eng/pipelines/templates/jobs/ci.tests.yml
# eng/pipelines/templates/jobs/ci.yml
# eng/pipelines/templates/jobs/live.test.yml

# Please use `$(TargetingString)` to refer to the python packages glob string. This was previously `${{ parameters.BuildTargetingString }}`.
steps:
- template: /eng/pipelines/templates/steps/analyze_dependency.yml

Expand All @@ -25,16 +30,6 @@ steps:
ServiceName: ${{parameters.ServiceDirectory}}
ForRelease: false

# - template: /eng/common/pipelines/templates/steps/verify-samples.yml
# parameters:
# ServiceDirectory: ${{parameters.ServiceDirectory}}

# Using --always-succeed so as not to block the build. Once package
# target is based on data available per-package the --always-succeed should
# be removed so this script can help enforce correct practices
# (https://github.com/Azure/azure-sdk-for-python/issues/8697)


- pwsh: |
pip install -r eng/ci_tools.txt $(if($IsWindows) {"--user" })
displayName: 'Install Necessary Dependencies'
Expand Down Expand Up @@ -83,31 +78,28 @@ steps:
condition: and(succeededOrFailed(), ne(variables['Skip.VerifySdist'],'true'))
inputs:
scriptPath: 'scripts/devops_tasks/setup_execute_tests.py'
arguments: '"${{ parameters.BuildTargetingString }}" --service=${{parameters.ServiceDirectory}} --toxenv=verifysdist'
arguments: '"$(TargetingString)" --service=${{parameters.ServiceDirectory}} --toxenv=verifysdist'

- task: PythonScript@0
displayName: 'Verify whl'
condition: and(succeededOrFailed(), ne(variables['Skip.VerifyWhl'],'true'))
inputs:
scriptPath: 'scripts/devops_tasks/setup_execute_tests.py'
arguments: '"${{ parameters.BuildTargetingString }}" --service=${{parameters.ServiceDirectory}} --toxenv=verifywhl'
arguments: '"$(TargetingString)" --service=${{parameters.ServiceDirectory}} --toxenv=verifywhl'

- template: run_mypy.yml
parameters:
ServiceDirectory: ${{ parameters.ServiceDirectory }}
BuildTargetingString: ${{ parameters.BuildTargetingString }}
TestMarkArgument: ${{ parameters.TestMarkArgument }}

- template: run_pylint.yml
parameters:
ServiceDirectory: ${{ parameters.ServiceDirectory }}
BuildTargetingString: ${{ parameters.BuildTargetingString }}

- ${{ if parameters.ValidateFormatting }}:
- template: run_black.yml
parameters:
ServiceDirectory: ${{ parameters.ServiceDirectory }}
BuildTargetingString: ${{ parameters.BuildTargetingString }}
ValidateFormatting: ${{ parameters.ValidateFormatting }}

- task: DownloadPipelineArtifact@2
Expand All @@ -119,21 +111,18 @@ steps:
- template: ../steps/run_apistub.yml
parameters:
ServiceDirectory: ${{ parameters.ServiceDirectory }}
BuildTargetingString: ${{ parameters.BuildTargetingString }}
TestMarkArgument: ${{ parameters.TestMarkArgument }}
AdditionalTestArgs: ${{parameters.AdditionalTestArgs}}

- template: ../steps/run_bandit.yml
parameters:
ServiceDirectory: ${{ parameters.ServiceDirectory }}
BuildTargetingString: ${{ parameters.BuildTargetingString }}
TestMarkArgument: ${{ parameters.TestMarkArgument }}
AdditionalTestArgs: ${{parameters.AdditionalTestArgs}}

- template: ../steps/run_breaking_changes.yml
parameters:
ServiceDirectory: ${{ parameters.ServiceDirectory }}
BuildTargetingString: ${{ parameters.BuildTargetingString }}
TestMarkArgument: ${{ parameters.TestMarkArgument }}
AdditionalTestArgs: ${{parameters.AdditionalTestArgs}}

Expand Down
19 changes: 12 additions & 7 deletions eng/pipelines/templates/steps/build-artifacts.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,6 @@ parameters:
- name: TestPipeline
type: boolean
default: false
- name: BuildTargetingString
type: string
default: 'azure-*'
- name: ServiceDirectory
type: string
default: ''
Expand All @@ -18,13 +15,21 @@ parameters:
type: object
default: []

# The variable TargetingString is set by template `eng/pipelines/templates/steps/targeting-string-resolve.yml`. This template is invoked from yml files:
# eng/pipelines/templates/jobs/ci.tests.yml
# eng/pipelines/templates/jobs/ci.yml
# eng/pipelines/templates/jobs/live.test.yml

# Please use `$(TargetingString)` to refer to the python packages glob string. This was previously `${{ parameters.BuildTargetingString }}`.
steps:
- template: /eng/common/pipelines/templates/steps/set-test-pipeline-version.yml
parameters:
PackageName: "azure-template"
ServiceDirectory: "template"
TestPipeline: ${{ parameters.TestPipeline }}

- template: /eng/common/pipelines/templates/steps/set-default-branch.yml

- script: |
echo "##vso[build.addbuildtag]Scheduled"
displayName: 'Tag scheduled builds'
Expand All @@ -43,7 +48,7 @@ steps:
displayName: 'Generate Python2 Applicable Namespace Packages'
inputs:
scriptPath: 'scripts/devops_tasks/build_packages.py'
arguments: '-d "$(Build.ArtifactStagingDirectory)" "${{ parameters.BuildTargetingString }}" --pkgfilter="nspkg" --service=${{parameters.ServiceDirectory}}'
arguments: '-d "$(Build.ArtifactStagingDirectory)" "$(TargetingString)" --pkgfilter="nspkg" --service=${{parameters.ServiceDirectory}}'

- task: UsePythonVersion@0
displayName: 'Use Python $(PythonVersion)'
Expand All @@ -57,7 +62,7 @@ steps:
- template: ../steps/set-dev-build.yml
parameters:
ServiceDirectory: ${{ parameters.ServiceDirectory }}
BuildTargetingString: ${{ parameters.BuildTargetingString }}

- task: Powershell@2
inputs:
filePath: $(Build.SourcesDirectory)/eng/common/scripts/Save-Package-Properties.ps1
Expand All @@ -73,7 +78,7 @@ steps:
displayName: 'Generate Packages'
inputs:
scriptPath: 'scripts/devops_tasks/build_packages.py'
arguments: '-d "$(Build.ArtifactStagingDirectory)" "${{ parameters.BuildTargetingString }}" --service=${{parameters.ServiceDirectory}} --devbuild="$(SetDevVersion)"'
arguments: '-d "$(Build.ArtifactStagingDirectory)" "$(TargetingString)" --service=${{parameters.ServiceDirectory}} --devbuild="$(SetDevVersion)"'

- script: |
twine check $(Build.ArtifactStagingDirectory)/**/*.whl
Expand All @@ -86,7 +91,7 @@ steps:
inputs:
scriptPath: 'scripts/devops_tasks/setup_execute_tests.py'
arguments: >-
"${{ parameters.BuildTargetingString }}"
"$(TargetingString)"
--service="${{ parameters.ServiceDirectory }}"
--toxenv=sphinx

Expand Down
12 changes: 9 additions & 3 deletions eng/pipelines/templates/steps/build-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,22 @@ parameters:
BeforeTestSteps: []
AfterTestSteps: []
CoverageArg: ''
BuildTargetingString: 'azure-*'
ToxTestEnv: ""
RunCoverage: ne(variables['CoverageArg'], '--disablecov')
ToxEnvParallel: ''
InjectedPackages: ''
DevFeedName: 'public/azure-sdk-for-python'

# The variable TargetingString is set by template `eng/pipelines/templates/steps/targeting-string-resolve.yml`. This template is invoked from yml files:
# eng/pipelines/templates/jobs/ci.tests.yml
# eng/pipelines/templates/jobs/ci.yml
# eng/pipelines/templates/jobs/live.test.yml

# Please use `$(TargetingString)` to refer to the python packages glob string. This was previously `${{ parameters.BuildTargetingString }}`.
steps:
- pwsh: |
gci -r $(Build.ArtifactStagingDirectory)
displayName: Dump Artifact Directory

- template: /eng/pipelines/templates/steps/use-python-version.yml
parameters:
Expand All @@ -43,7 +49,7 @@ steps:
inputs:
scriptPath: 'scripts/devops_tasks/setup_execute_tests.py'
arguments: >-
"${{ parameters.BuildTargetingString }}"
"$(TargetingString)"
${{ parameters.AdditionalTestArgs }}
${{ parameters.CoverageArg }}
--mark_arg="${{ parameters.TestMarkArgument }}"
Expand Down Expand Up @@ -94,7 +100,7 @@ steps:
inputs:
scriptPath: 'scripts/devops_tasks/setup_execute_tests.py'
arguments: >-
"${{ parameters.BuildTargetingString }}"
"$(TargetingString)"
--service="${{ parameters.ServiceDirectory }}"
--toxenv="samples"
env: ${{ parameters.EnvVars }}
9 changes: 7 additions & 2 deletions eng/pipelines/templates/steps/run_apistub.yml
Original file line number Diff line number Diff line change
@@ -1,17 +1,22 @@
parameters:
BuildTargetingString: 'azure-*'
ServiceDirectory: ''
TestMarkArgument: ''
AdditionalTestArgs: ''

# The variable TargetingString is set by template `eng/pipelines/templates/steps/targeting-string-resolve.yml`. This template is invoked from yml files:
# eng/pipelines/templates/jobs/ci.tests.yml
# eng/pipelines/templates/jobs/ci.yml
# eng/pipelines/templates/jobs/live.test.yml

# Please use `$(TargetingString)` to refer to the python packages glob string. This was previously `${{ parameters.BuildTargetingString }}`.
steps:
- task: PythonScript@0
displayName: 'Run Api Stub Generation'
condition: and(succeededOrFailed(), ne(variables['Skip.ApiStubGen'],'true'))
inputs:
scriptPath: 'scripts/devops_tasks/setup_execute_tests.py'
arguments: >-
"${{ parameters.BuildTargetingString }}"
"$(TargetingString)"
${{ parameters.AdditionalTestArgs }}
--mark_arg="${{ parameters.TestMarkArgument }}"
--service="${{ parameters.ServiceDirectory }}"
Expand Down
9 changes: 7 additions & 2 deletions eng/pipelines/templates/steps/run_bandit.yml
Original file line number Diff line number Diff line change
@@ -1,16 +1,21 @@
parameters:
BuildTargetingString: 'azure-*'
ServiceDirectory: ''
TestMarkArgument: ''
EnvVars: {}

# The variable TargetingString is set by template `eng/pipelines/templates/steps/targeting-string-resolve.yml`. This template is invoked from yml files:
# eng/pipelines/templates/jobs/ci.tests.yml
# eng/pipelines/templates/jobs/ci.yml
# eng/pipelines/templates/jobs/live.test.yml

# Please use `$(TargetingString)` to refer to the python packages glob string. This was previously `${{ parameters.BuildTargetingString }}`.
steps:
- task: PythonScript@0
displayName: 'Run Bandit'
inputs:
scriptPath: 'scripts/devops_tasks/setup_execute_tests.py'
arguments: >-
"${{ parameters.BuildTargetingString }}"
"$(TargetingString)"
--mark_arg="${{ parameters.TestMarkArgument }}"
--service="${{ parameters.ServiceDirectory }}"
--toxenv="bandit"
Expand Down
1 change: 0 additions & 1 deletion eng/pipelines/templates/steps/run_black.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
parameters:
BuildTargetingString: 'azure-*'
ServiceDirectory: ''
ValidateFormatting: false
EnvVars: {}
Expand Down
Loading