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

[functionapp] Add support for v3 function apps and node 12. #11987

Merged
merged 3 commits into from Feb 13, 2020

Conversation

@gzuber
Copy link
Member

gzuber commented Jan 30, 2020

Fixes #11349
Fixes #11364
Fixes #11813


This checklist is used to make sure that common guidelines for a pull request are followed.

  • The PR has modified HISTORY.rst describing any customer-facing, functional changes. Note that this does not include changes only to help content. (see Modifying change log).

  • I adhere to the Command Guidelines.

@gzuber gzuber requested review from Juliehzl and qwordy as code owners Jan 30, 2020
@yonzhan yonzhan added this to the S165 milestone Jan 30, 2020
@yonzhan

This comment has been minimized.

Copy link
Contributor

yonzhan commented Jan 30, 2020

add to S165.

@yonzhan yonzhan requested review from MyronFanQiu and jiasli Jan 30, 2020
@gzuber gzuber force-pushed the gzuber:11349_v3_function_apps branch from 68838df to 4ba2467 Jan 30, 2020
Copy link
Contributor

Hazhzeng left a comment

nit: let's simplify this code piece a bit. The rest looks good to me.

runtime_versions_list = list(runtime_versions)
runtime_versions_list.sort(key=float)
Comment on lines +63 to +62

This comment has been minimized.

Copy link
@Hazhzeng

Hazhzeng Jan 30, 2020

Contributor

The set will perform auto sort, we don't need to do this explicitly.
The next line ', '.join will iterate through the set. So we may not need these two lines.

This comment has been minimized.

Copy link
@gzuber

gzuber Jan 30, 2020

Author Member

Yeah so this was a bit frustrating because if I just join the set, it'll print out node versions as "8, 12, 10" for some reason. I can't sort the set so if I turn it into a list and sort it, it'll print "10, 12, 8" because 1 comes before 8. Setting the key to float will sort them as if they're floats and behave how we would want.

This comment has been minimized.

Copy link
@Hazhzeng

Hazhzeng Jan 30, 2020

Contributor

OK, I see, yea, that makes sense. We can keep them here.

@gzuber gzuber force-pushed the gzuber:11349_v3_function_apps branch 2 times, most recently from b382267 to 83829bf Jan 30, 2020
@gzuber

This comment has been minimized.

Copy link
Member Author

gzuber commented Feb 1, 2020

@qwordy It looks like my tests are failing due to the same issue fixed by #12006. Is the hotfix going to be merged to dev?

@gzuber gzuber force-pushed the gzuber:11349_v3_function_apps branch from 83829bf to 6d1a57e Feb 1, 2020
@jongio

This comment has been minimized.

Copy link
Member

jongio commented Feb 1, 2020

This is highly needed as the only way it appears to upgrade to ~3 is via the Portal. I spent a bunch of time with this today. Thank you.

@yonzhan yonzhan requested a review from arrownj Feb 1, 2020
@gzuber gzuber force-pushed the gzuber:11349_v3_function_apps branch from 6d1a57e to 27246d8 Feb 3, 2020
@jongio

This comment has been minimized.

Copy link
Member

jongio commented Feb 3, 2020

Thanks @gzuber, looking good.

@gzuber gzuber force-pushed the gzuber:11349_v3_function_apps branch from 27246d8 to 6f9f353 Feb 4, 2020
@gzuber

This comment has been minimized.

Copy link
Member Author

gzuber commented Feb 5, 2020

@qwordy @yonzhan @Juliehzl This should be ready to review whenever you get a chance, thanks!

@Hazhzeng

This comment has been minimized.

Copy link
Contributor

Hazhzeng commented Feb 5, 2020

Hi @qwordy and @Juliehzl,

Could you take a look at this PR, we want to have it in the next sprint (S165).
Thanks in advance

@yonzhan

This comment has been minimized.

Copy link
Contributor

yonzhan commented Feb 6, 2020

@qwordy will help merge this PR for S165.

@@ -460,6 +468,7 @@ def load_arguments(self, _):
help='Provide a string value of a Storage Account in the provided Resource Group. Or Resource ID of a Storage Account in a different Resource Group')
c.argument('consumption_plan_location', options_list=['--consumption-plan-location', '-c'],
help="Geographic location where Function App will be hosted. Use `az functionapp list-consumption-locations` to view available locations.")
c.argument('version', options_list=['--version', '-v'], help='The functions runtime version.', arg_type=get_enum_type(HOST_VERSIONS_FUNCTIONAPP), configured_default='2')
c.argument('runtime', help='The functions runtime stack.', arg_type=get_enum_type(set(LINUX_RUNTIMES).union(set(WINDOWS_RUNTIMES))))
c.argument('runtime_version', help='The version of the functions runtime stack. '
'Allowed values for each --runtime are: ' + ', '.join(functionapp_runtime_to_version_texts))

This comment has been minimized.

Copy link
@qwordy

qwordy Feb 6, 2020

Member

Can we put all available versions here and delete --version? What's the difference between version 2 and 3 when choosing same runtime and runtime_version?

This comment has been minimized.

Copy link
@gzuber

gzuber Feb 6, 2020

Author Member

runtime_version is the version of runtime language you're using (node, python, etc...) whereas version is the version of the functions host you want to use (You can make a Functions v2 App or v3 App). The two aren't related other than each functions host version only supports a subset of runtime versions. We recently GA'd functions host v3 (more info here) so we need to allow customers a choice for their host version.

This comment has been minimized.

Copy link
@jongio

jongio Feb 6, 2020

Member

To chime in...it took me a minute to figure this out when I was first ramping up. Maybe we scrub docs to ensure that we clearly describe the differences.

This comment has been minimized.

Copy link
@qwordy

qwordy Feb 7, 2020

Member

I see. Thanks for the detailed explanation.

@@ -460,6 +468,7 @@ def load_arguments(self, _):
help='Provide a string value of a Storage Account in the provided Resource Group. Or Resource ID of a Storage Account in a different Resource Group')
c.argument('consumption_plan_location', options_list=['--consumption-plan-location', '-c'],
help="Geographic location where Function App will be hosted. Use `az functionapp list-consumption-locations` to view available locations.")
c.argument('version', options_list=['--version', '-v'], help='The functions runtime version.', arg_type=get_enum_type(HOST_VERSIONS_FUNCTIONAPP), configured_default='2')

This comment has been minimized.

Copy link
@qwordy

qwordy Feb 6, 2020

Member

Maybe more description about version 2 and 3. I don't know which one I should choose.

@qwordy
qwordy approved these changes Feb 7, 2020
gzuber added 2 commits Feb 12, 2020
…s. Added functions version to invalid runtime version error.
@gzuber

This comment has been minimized.

Copy link
Member Author

gzuber commented Feb 12, 2020

@qwordy -- we decided to rename the --version flag in an effort to clear up some of the confusion we saw on this PR. It's now called --functions-version and I think the help text is a little more clear. Thanks for your input on that! This PR should be good to be merged now for S165 (I don't have permissions)

@qwordy

This comment has been minimized.

Copy link
Member

qwordy commented Feb 13, 2020

@qwordy -- we decided to rename the --version flag in an effort to clear up some of the confusion we saw on this PR. It's now called --functions-version and I think the help text is a little more clear. Thanks for your input on that! This PR should be good to be merged now for S165 (I don't have permissions)

I think both parameter names are good as long as comprehensible help messages are available. Thanks for your update. I'll merge it now.

@qwordy qwordy merged commit 9ac96e9 into Azure:dev Feb 13, 2020
30 checks passed
30 checks passed
Azure.azure-cli Build #20200212.2 succeeded
Details
Azure.azure-cli (Build Docker Image) Build Docker Image succeeded
Details
Azure.azure-cli (Build Python Wheels) Build Python Wheels succeeded
Details
Azure.azure-cli (Build Windows MSI) Build Windows MSI succeeded
Details
Azure.azure-cli (Check CLI Linter) Check CLI Linter succeeded
Details
Azure.azure-cli (Check CLI Style) Check CLI Style succeeded
Details
Azure.azure-cli (Check License, History, and DocMap) Check License, History, and DocMap succeeded
Details
Azure.azure-cli (Check Module Load Performance) Check Module Load Performance succeeded
Details
Azure.azure-cli (Check the Format of Pull Request Title) Check the Format of Pull Request Title succeeded
Details
Azure.azure-cli (Credential Scan) Credential Scan succeeded
Details
Azure.azure-cli (Extract Metadata) Extract Metadata succeeded
Details
Azure.azure-cli (Integration Test against Profiles Python36) Integration Test against Profiles Python36 succeeded
Details
Azure.azure-cli (Integration Test against Profiles Python38) Integration Test against Profiles Python38 succeeded
Details
Azure.azure-cli (Run Automation Python 3.8, Profile 2018-03-01) Run Automation Python 3.8, Profile 2018-03-01 succeeded
Details
Azure.azure-cli (Run Automation Reduced Python 3.6) Run Automation Reduced Python 3.6 succeeded
Details
Azure.azure-cli (Run Automation Reduced Python 3.6, Profile 2019-03-01) Run Automation Reduced Python 3.6, Profile 2019-03-01 succeeded
Details
Azure.azure-cli (Run Automation Reduced Python 3.8) Run Automation Reduced Python 3.8 succeeded
Details
Azure.azure-cli (Run Automation Reduced Python 3.8, Profile 2019-03-01) Run Automation Reduced Python 3.8, Profile 2019-03-01 succeeded
Details
Azure.azure-cli (Run Automation, Profile 2018-03-01) Run Automation, Profile 2018-03-01 succeeded
Details
Azure.azure-cli (Test Docker Image) Test Docker Image succeeded
Details
Azure.azure-cli (Test Extensions Loading Python36) Test Extensions Loading Python36 succeeded
Details
Azure.azure-cli (Test Python Wheels) Test Python Wheels succeeded
Details
Azure.azure-cli (Unit Test for Core and Telemetry Python36) Unit Test for Core and Telemetry Python36 succeeded
Details
Azure.azure-cli (Unit Test for Core and Telemetry Python38) Unit Test for Core and Telemetry Python38 succeeded
Details
Azure.azure-cli (Verify Sphinx Document Generator) Verify Sphinx Document Generator succeeded
Details
Azure.azure-cli (Verify src/azure-cli/requirements.*.Darwin.txt) Verify src/azure-cli/requirements.*.Darwin.txt succeeded
Details
Azure.azure-cli (Verify src/azure-cli/requirements.*.Linux.txt) Verify src/azure-cli/requirements.*.Linux.txt succeeded
Details
Azure.azure-cli (Verify src/azure-cli/requirements.*.Windows.txt) Verify src/azure-cli/requirements.*.Windows.txt succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla All CLA requirements met.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.