[functionapp] Enable server side build in OryxBuilder #45
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ankitkumarr
reviewed
May 24, 2019
| return Task.CompletedTask; | ||
| // Detect if package upload is necessary for server side build | ||
| if (artifactPath != null) | ||
| { |
There was a problem hiding this comment.
Please correct me if I am wrong, but looks like artifactPath will never actually be null for any function app. So, this path will always be hit, whether it's consumption or dedicated. We probably only want to hit this condition if we are in Consumption I think. Should we do a check for FunctionAppHelper.HasScmRunFromPackage() here?
ankitkumarr
reviewed
May 24, 2019
| /* TODO: Hazhzeng, uncomment this when "RemoveAllStandardWorker" is out in ANT 83 | ||
| * string baseUrl = $"http://{websiteHostname}/operations/removeworker/{sitename}/allStandard?token={authTokenEncoded}"; | ||
| */ | ||
| string baseUrl = $"http://{websiteHostname}/operations/removeworker/{sitename}/all?token={authTokenEncoded}"; |
There was a problem hiding this comment.
The internal call only accepts http protocol. Tried with https, failed to create SSL channel :(
|
|
||
| // | ||
| // Run express build setups if needed | ||
| if (args.Flags == BuildOptimizationsFlags.UseExpressBuild) | ||
| { | ||
| if (FunctionAppHelper.LooksLikeFunctionApp()) | ||
| { | ||
| SetupFunctionAppExpressArtifacts(context); | ||
| artifactPath = SetupFunctionAppExpressArtifacts(context); |
There was a problem hiding this comment.
SetupFunctionAppExpressArtifacts sets packaname.txt and packagepath.txt as well. We probably don't need that for consumption. Maybe we just do the packaging for Consumption, and not use BUILD_FLAGS.
Hazhzeng
force-pushed
the
hazeng-pr4-scmbuild
branch
from
b71a71e to
ffd2a4b
Compare
Hazhzeng
force-pushed
the
hazeng-pr4-scmbuild
branch
from
842edc3 to
641f863
Compare
sanchitmehta
approved these changes
Jun 13, 2019
| @@ -37,7 +37,7 @@ public LinuxConsumptionInstanceAdminController(ILinuxConsumptionInstanceManager | |||
| /// </summary> | |||
| /// <returns>Expect 200 when current service is up and running</returns> | |||
| [HttpGet] | |||
| [Authorize(Policy = AuthPolicyNames.AdminAuthLevel)] | |||
| //[Authorize(Policy = AuthPolicyNames.AdminAuthLevel)] | |||
There was a problem hiding this comment.
Do we want to disable the auth here?
added 7 commits
June 13, 2019 11:38
Fix using directives Add TODO to remove all standard workers Address issues in comment Rename Linux consumption functionapp related function Internal call does not require https Update remove all worker URL Catch error when SCM_RUN_FROM_PACKAGE is not provided
Hazhzeng
force-pushed
the
hazeng-pr4-scmbuild
branch
from
1ac11e0 to
2ab259f
Compare
sanchitmehta
approved these changes
Jun 14, 2019
Hazhzeng
pushed a commit
to Hazhzeng/KuduLite
that referenced
this pull request
Jul 20, 2021
…ice#45) * Enable scm build in oryx builder Fix using directives Add TODO to remove all standard workers Address issues in comment Rename Linux consumption functionapp related function Internal call does not require https Update remove all worker URL Catch error when SCM_RUN_FROM_PACKAGE is not provided * Add unit tests for Oryx Factory * Make SetupLinuxConsumptionFunctionAppDeployment(context).Wait() synchronous * Remove assignment auth * Move ScmRunFromPackage from AppSettings to Environment Variables * Null check for pushdeploymentcontroller.cs * Reenable Authorize * Address unit tests
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
CR Request
@ankitkumarr @maiqbal11 @sanchitmehta