Updating coretools to detect unsupported inproc6 sdk scenario #3825

surgupta-msft · 2024-09-17T19:54:13Z

Issue describing the changes in this PR

Our goal is to detect if OOP host is getting inproc payload and log the message to update the sdk to the latest and exit. Discussion in comment - #3825 (comment)

File artifactsconfig.json - It's a new config file we are introducing with this PR - it may or may not be present in coretools. Any changes that should go to visual studio and not to cli/vscode can have this check - IsMinifiedVersion() = true. In BuildSteps.cs, we create minified version of coretools for Visual Studio case - link.

For now, artifactsconfig.json has only below content -

{
    "minifiedVersion" : true
}

Pull request checklist

My changes do not require documentation changes
- Otherwise: Documentation issue linked to PR - Create documentation for Core tools requirement on minimum in-proc SDK version #3817
My changes do not need to be backported to a previous version
- Otherwise: Backport tracked by issue/PR #issue_or_pr
I have added all required tests (Unit tests, E2E tests)

mattchenderson

Should this PR be targeting the other PR branch? We should land this, as it will release ahead of the OOP Core Tools changes, right?

src/Azure.Functions.Cli/Actions/HostActions/StartHostAction.cs

test/Azure.Functions.Cli.Tests/E2E/StartTests.cs

src/Azure.Functions.Cli/Actions/HostActions/StartHostAction.cs

fabiocav · 2024-09-19T23:36:29Z

Should this PR be targeting the other PR branch? We should land this, as it will release ahead of the OOP Core Tools changes, right?

Discussed this with @mattchenderson and we're good with the changes against the feature branch. This will give us the ability to update the SDK first while retaining support for in-proc with the default version of Core Tools.

test/Azure.Functions.Cli.Tests/E2E/StartTests.cs

src/Azure.Functions.Cli/Actions/HostActions/StartHostAction.cs

src/Azure.Functions.Cli/Azure.Functions.Cli.csproj

src/Azure.Functions.Cli/Actions/HostActions/StartHostAction.cs

src/Azure.Functions.Cli/ArtifactsConfigurationBuilder.cs

src/Azure.Functions.Cli/Common/Utilities.cs

jviau

Won't block this on the details of using IConfigurationBuilder here. Maybe we can revisit later.

src/Azure.Functions.Cli/ArtifactsConfigurationBuilder.cs

jviau · 2024-09-30T22:55:17Z

I'd like to discuss why we are using a config file for something that I assume is constant for a given set of artifacts. Curious why this path was chosen over embedding constants into the assembly for the min versions?

@fabiocav @surgupta-msft

Update: discussed offline - we do not produce separate binaries for the minified versions, instead we repackage existing binaries. Using a compiler flag would be out of scope at this time.

mattchenderson

Why is a separate issue and PR needed for generation of the new configuration file? Shouldn't that be a part of this PR as well?

surgupta-msft · 2024-10-02T22:55:16Z

Why is a separate issue and PR needed for generation of the new configuration file? Shouldn't that be a part of this PR as well?

Thought the PR would be merged in the last sprint and did not want to increase the scope of it but since we are already halfway in the current sprint, it makes sense to include build side changes as well. I have updated the PR with changes in BuildSteps.cs

* Core Tools OOP Host (#3802) * this doesn't work * this finally works now * default should be oop host * added edge cases * fixing formatting * using determineTargetFramework * saving tests * adding copy step for OOP * fixing the webhost reference * removing test * setting host version in ps script * update to validate worker versions script * update versions for worker packages * adding ToString * validating worker version * validate worker versions * updating csproj to compile * addressing pr feedback * updating build steps * adding build step * fixing build step * trying to get this working * reverrting target runtimes * updating tests * adding dotnet info step * adding changes * removing extra test * trying to specify architecture * modifying tests to see if they work * narrowing down to tests that are failing * trying to see if it works with nobuild flag * addressing pr feedback * updating tests with latest logging * addressing comments and marking flaky tests * updating so that we are only using net8 framework * pushing change for branch build * adding single quotes * reverting quotes * adding code mirror fiile * updating build step * updatinng build steps * updating build step * adding step for dotnet publish * set inprochost compilation system to diff value and skip flaky test * updating public build pipeline to trigger * adding extra changes for pipeline * public build yml * updating official build * readd net8 build artifact step * readding space back * addressing initial comments * adding explicit openTelemetry dlls * simplifying logic of startHostAction * addressing PR feedback * start tests * changing some of the tests back * reverting test back to normal * fixing spacing for csproj * addressing PR feedback * adding extra variable * adding logic for edge case scenarios * added edge cases tests * removing extra line in node * addressing comments * moving validate host runtime to its own method * forgot to add return statement * Custom host changes for VS Scenario (#3831) * this doesn't work * this finally works now * default should be oop host * added edge cases * fixing formatting * using determineTargetFramework * saving tests * adding copy step for OOP * fixing the webhost reference * removing test * setting host version in ps script * update to validate worker versions script * update versions for worker packages * adding ToString * validating worker version * validate worker versions * updating csproj to compile * addressing pr feedback * updating build steps * adding build step * fixing build step * trying to get this working * reverrting target runtimes * updating tests * adding dotnet info step * adding changes * removing extra test * trying to specify architecture * modifying tests to see if they work * narrowing down to tests that are failing * trying to see if it works with nobuild flag * addressing pr feedback * updating tests with latest logging * addressing comments and marking flaky tests * updating so that we are only using net8 framework * pushing change for branch build * adding single quotes * reverting quotes * adding code mirror fiile * updating build step * updatinng build steps * updating build step * adding step for dotnet publish * set inprochost compilation system to diff value and skip flaky test * updating public build pipeline to trigger * adding extra changes for pipeline * public build yml * updating official build * readd net8 build artifact step * readding space back * addressing initial comments * adding explicit openTelemetry dlls * simplifying logic of startHostAction * addressing PR feedback * start tests * changing some of the tests back * reverting test back to normal * fixing spacing for csproj * addressing PR feedback * adding extra variable * adding logic for edge case scenarios * added edge cases tests * initial changes * removing extra line in node * removing files not needed for custom host * adding build steps * addressing comments * moving validate host runtime to its own method * forgot to add return statement * trying out creating directory * remove skip artifact gen * updating build step * updating build to include in official pipeline * updating publishing custom host step * code mirror * trying to get official build working * program cs update * updating custom host to work with parsing local.settings.json * console read line for stalling process * removing changes not needed for custom host * only including inproc6 and inproc8 changes * addressing initial comments * removing skip artifact gen flag * verbose logging * tryna add release pipeline * windows yaml changes * addressing comments about yml * updating log message * fixing spacing * fixing spacing again * trying spacing again * spacing change * tryan fix spacing this way * spacing * spacing * new line * regular indentation * super indent * changing spacing back to normal * addressing comments * fixing pipeline * Fix build to remove uploadToStorage (#3839) * build steps * trying to parallelize build steps * Revert "build steps" This reverts commit 9aa82fa. * Reapply "build steps" This reverts commit a7b7d46. * Revert "trying to parallelize build steps" This reverts commit d10ef5c. * Adding artifact assembler tool which consolidates the artifacts and produce the package (#3843) * Adding artifact assembler program which collects artifacts from different sources and assemble the final package for core tools. * Adding README * Remove downloaded artifact directories. Minor language update to README * Updating the artifact name to include "_inproc" suffix (#3849) * Small refactor and new tests (#3835) * Updating coretools to detect unsupported inproc6 sdk scenario (#3825) * Restructuring custom host to improve error handling (#3861) * Create Artifacts for releasing OOP core tools (#3848) * assemble artifacts * adding code mirror to test * try awaiting tasks to save space * removing --net8 dependency * trying to delete directory one at a time * trying again with extracting * diff build steps for vs and cli * forgot to include other step for var * trying out new changes * clean up each zip file after extracting * trying another approach * try to delete each directory after copying * removing logic for creating zips rn * including e2e test changes and add powershell file * removing multi targeting * pass in staging directory * adding param * adding logging * printing out paths * adding isolated e2e tests and changing verbosity for logger * adding a new label * comments * adding more clarifying comments * removing extra test and types * adding logging statement for failing task if we fail tests * updating tests to only run in winx64 and winx86 * target directory * target directory * changes to msi file * print out directory * resolving path * reorg files and try to set env variable * dont delete artifacts * update generate msi files * add doller sign for variable name and fix e2e test * changing runtime version * updating to get correct cli version * lol forgot to have content in zip ps script * Update src/Azure.Functions.ArtifactAssembler/ArtifactAssembler.cs Co-authored-by: Lilian Kasem <[email protected]> * Update src/Azure.Functions.ArtifactAssembler/ArtifactAssembler.cs Co-authored-by: Lilian Kasem <[email protected]> * Update src/Azure.Functions.ArtifactAssembler/ArtifactAssembler.cs Co-authored-by: Lilian Kasem <[email protected]> * Update src/Azure.Functions.ArtifactAssembler/ArtifactAssembler.cs Co-authored-by: Lilian Kasem <[email protected]> * Update src/Azure.Functions.ArtifactAssembler/ArtifactAssembler.cs Co-authored-by: Lilian Kasem <[email protected]> * addressing comments * reverting programcs back to the way it was * staging directory not correct * merging * Revert "merging" This reverts commit 89cc110. * fixing spacing * wrong directory * trying to free up disk space * removing custom code mirror * removing dollar signs * updating build number * adding variables back * fix build number * fixing candle * fixing msi files * reverting files back to normal --------- Co-authored-by: Lilian Kasem <[email protected]> * Adding package reference (#3872) * adding system.text.json * update patch * move to 8.0.10 * fixing indentation * trying to change version * Templates version update (#3883) * Create metadata.json file in conslidated artifacts intermediate pipeline (#3884) * adding generate metadata file * remove code mirror * get correct inproc version * print out metadata file * fixing zip issue * fixing release number * fixing release number again * removing zip artifact script and fixing code mirror * template update (#4151) * Update templates for dotnet isolated worker 2.x (#4189) * adding tests for custom host * removing zipping for now * trying to print out value * update artifact assembler * removing extra delete file lol * adding env variables * add language * trying to see if this works for e2e tests * this should work * setting language * trying out new change * trying to set env variable * updating start tests * setting env variable * updating cli tester * trying to set testVsArtifacts * setting paths back to normal * local settings json parser print current directory * adding local.settings.json * tests should fail now if they actually fail * adding net6 inproc tests * updating path * start tests net6 update * updating zipping to happen * updating test project * fixing e2e test project * add trailing comma and test comment * updating tests to not exit with expected error * adding tests for custom host * removing zipping for now * trying to print out value * update artifact assembler * removing extra delete file lol * adding env variables * add language * trying to see if this works for e2e tests * this should work * setting language * trying out new change * trying to set env variable * updating start tests * setting env variable * updating cli tester * trying to set testVsArtifacts * setting paths back to normal * local settings json parser print current directory * adding local.settings.json * tests should fail now if they actually fail * adding net6 inproc tests * updating path * start tests net6 update * updating zipping to happen * updating test project * fixing e2e test project * add trailing comma and test comment * updating tests to not exit with expected error * updating artifact assembler * forgot to update assembler * changing environment variable back to normal * addressing comments * addressing comments * deleting extra test csproj folder entry and adding comments to pipeline helpers * adding osx support (#4213) * removing duplicate code * using net8 binaries instead of net6 for custom host osx * addressing feedback for method name --------- Co-authored-by: Shyju Krishnankutty <[email protected]> Co-authored-by: Fabio Cavalcante <[email protected]> Co-authored-by: Surbhi Gupta <[email protected]> Co-authored-by: Matthew Henderson <[email protected]> Co-authored-by: Lilian Kasem <[email protected]> Co-authored-by: Jacob Viau <[email protected]>

surgupta-msft requested review from kshyju, liliankasem and aishwaryabh September 17, 2024 21:44

surgupta-msft marked this pull request as ready for review September 18, 2024 18:33

surgupta-msft requested a review from a team as a code owner September 18, 2024 18:33

liliankasem approved these changes Sep 19, 2024

View reviewed changes

mattchenderson requested changes Sep 19, 2024

View reviewed changes

microsoft-github-policy-service bot added the Needs: Author Feedback label Sep 19, 2024

fabiocav reviewed Sep 19, 2024

View reviewed changes

src/Azure.Functions.Cli/Actions/HostActions/StartHostAction.cs Outdated Show resolved Hide resolved

mattchenderson linked an issue Sep 19, 2024 that may be closed by this pull request

Update Core Tools to detect unsupported in-proc workloads #3816

Closed

microsoft-github-policy-service bot removed the Needs: Author Feedback label Sep 20, 2024

Azure deleted a comment from fabiocav Sep 24, 2024

surgupta-msft requested review from mattchenderson, fabiocav and liliankasem September 24, 2024 18:49

Base automatically changed from aibhandari/core-tools-oop to feature/oop-host September 24, 2024 19:05

aishwaryabh requested a review from a team as a code owner September 24, 2024 19:05

surgupta-msft closed this Sep 24, 2024

surgupta-msft reopened this Sep 24, 2024

surgupta-msft commented Sep 25, 2024

View reviewed changes

test/Azure.Functions.Cli.Tests/E2E/StartTests.cs Outdated Show resolved Hide resolved

aishwaryabh reviewed Sep 25, 2024

View reviewed changes

src/Azure.Functions.Cli/Actions/HostActions/StartHostAction.cs Outdated Show resolved Hide resolved

aishwaryabh reviewed Sep 25, 2024

View reviewed changes

src/Azure.Functions.Cli/Actions/HostActions/StartHostAction.cs Show resolved Hide resolved

aishwaryabh reviewed Sep 25, 2024

View reviewed changes

src/Azure.Functions.Cli/Actions/HostActions/StartHostAction.cs Outdated Show resolved Hide resolved

aishwaryabh reviewed Sep 25, 2024

View reviewed changes

src/Azure.Functions.Cli/Azure.Functions.Cli.csproj Outdated Show resolved Hide resolved

fabiocav requested a review from soninaren September 25, 2024 20:27

surgupta-msft added 3 commits September 25, 2024 14:59

vlaidation changes

00ddf5f

Test fix

53023cc

Tests update

a3450b0

surgupta-msft force-pushed the surgupta/coretools-sdk-inproc branch from 2a6e3f8 to a3450b0 Compare September 25, 2024 22:07

surgupta-msft added 2 commits September 27, 2024 20:48

PR feedback

6cae371

empty line fix

ee76d64

jviau requested changes Sep 30, 2024

View reviewed changes

src/Azure.Functions.Cli/Actions/HostActions/StartHostAction.cs Outdated Show resolved Hide resolved

src/Azure.Functions.Cli/ArtifactsConfigurationBuilder.cs Outdated Show resolved Hide resolved

src/Azure.Functions.Cli/Common/Utilities.cs Outdated Show resolved Hide resolved

microsoft-github-policy-service bot added Needs: Author Feedback and removed Needs: Author Feedback labels Sep 30, 2024

surgupta-msft added 2 commits September 30, 2024 14:39

making config builder inline

412fc93

Method name update

98fb231

jviau approved these changes Sep 30, 2024

View reviewed changes

src/Azure.Functions.Cli/ArtifactsConfigurationBuilder.cs Outdated Show resolved Hide resolved

Removing artifact builder class

ef8ec80

This was referenced Oct 1, 2024

Update build flow for minified versions to include a configuration file #3854

Closed

Separate build for minified versions #3855

Closed

mattchenderson reviewed Oct 2, 2024

View reviewed changes

surgupta-msft added 3 commits October 1, 2024 22:40

merge with base

b7590c5

Updating build to add config file for the minified versions

e8bf35c

Merge with base

9a63517

surgupta-msft requested a review from mattchenderson October 2, 2024 22:46

mattchenderson linked an issue Oct 2, 2024 that may be closed by this pull request

Update build flow for minified versions to include a configuration file #3854

Closed

mattchenderson approved these changes Oct 3, 2024

View reviewed changes

surgupta-msft merged commit 141d259 into feature/oop-host Oct 3, 2024
4 checks passed

surgupta-msft deleted the surgupta/coretools-sdk-inproc branch October 3, 2024 05:07

kshyju pushed a commit that referenced this pull request Nov 4, 2024

Updating coretools to detect unsupported inproc6 sdk scenario (#3825)

83d9c84

aishwaryabh pushed a commit that referenced this pull request Nov 6, 2024

Updating coretools to detect unsupported inproc6 sdk scenario (#3825)

d25d55b

fabiocav pushed a commit that referenced this pull request Nov 6, 2024

Updating coretools to detect unsupported inproc6 sdk scenario (#3825)

1b49034

aishwaryabh pushed a commit that referenced this pull request Nov 25, 2024

Updating coretools to detect unsupported inproc6 sdk scenario (#3825)

baf28a5

aishwaryabh pushed a commit that referenced this pull request Nov 25, 2024

Updating coretools to detect unsupported inproc6 sdk scenario (#3825)

a87e50c

aishwaryabh pushed a commit that referenced this pull request Nov 25, 2024

Updating coretools to detect unsupported inproc6 sdk scenario (#3825)

24a5a28

aishwaryabh pushed a commit that referenced this pull request Nov 26, 2024

Updating coretools to detect unsupported inproc6 sdk scenario (#3825)

631f2e8

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updating coretools to detect unsupported inproc6 sdk scenario #3825

Updating coretools to detect unsupported inproc6 sdk scenario #3825

surgupta-msft commented Sep 17, 2024 •

edited

Loading

mattchenderson left a comment •

edited

Loading

fabiocav commented Sep 19, 2024

jviau left a comment

jviau commented Sep 30, 2024 •

edited

Loading

mattchenderson left a comment

surgupta-msft commented Oct 2, 2024

Updating coretools to detect unsupported inproc6 sdk scenario #3825

Updating coretools to detect unsupported inproc6 sdk scenario #3825

Conversation

surgupta-msft commented Sep 17, 2024 • edited Loading

Issue describing the changes in this PR

Pull request checklist

mattchenderson left a comment • edited Loading

Choose a reason for hiding this comment

fabiocav commented Sep 19, 2024

jviau left a comment

Choose a reason for hiding this comment

jviau commented Sep 30, 2024 • edited Loading

mattchenderson left a comment

Choose a reason for hiding this comment

surgupta-msft commented Oct 2, 2024

surgupta-msft commented Sep 17, 2024 •

edited

Loading

mattchenderson left a comment •

edited

Loading

jviau commented Sep 30, 2024 •

edited

Loading