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

Added pwsh az function container #153

Merged
merged 4 commits into from Nov 23, 2019
Merged

Added pwsh az function container #153

merged 4 commits into from Nov 23, 2019

Conversation

brettmillerb
Copy link
Contributor

@brettmillerb brettmillerb commented Nov 22, 2019

Good afternoon

I noticed that there was no Azure Functions PowerShell container available so I used the C# .NET core container and added Pwsh 6.2.3 to it in the dockerfile.

I'm by no means fluent with docker but I have tested this and it builds successfully but there is an error with setting the locale. Not sure if this is an ENV issue that needs to be resolved.

[106207 ms] perl: warning: Setting locale failed.
perl: warning: Please check that your locale settings:
        LANGUAGE = (unset),
        LC_ALL = "en_US.UTF-8",
        LANG = "en_US.UTF-8"
    are supported and installed on your system.
perl: warning: Falling back to the standard locale ("C").

The container still functions as normal and I can run and debug a local PowerShell Azure functions.

Please let me know if there are any changes required or if there's anything I have missed.

Copy link
Member

@TylerLeonhardt TylerLeonhardt left a comment

Looking good!

@Chuxel
Copy link
Member

@Chuxel Chuxel commented Nov 22, 2019

@brettmillerb Thanks so much for the contribution! That's awesome.

@TylerLeonhardt makes some great suggestions.

One other that I would add is we're typically not specifying the full version number in the dev container name unless we intend there to be more than one. Even then, it typically is a major release or version number that breaks compatibility. e.g. for .NET, that's 2.1.x, 2.2.x, 3.0.x, etc. For Node.js, its 10.x.x, 12.x.x. This allows people to still get back to the README when the minor or break-fix part of the version number is increased.

Perhaps this should be containers/azure-functions-pwsh-6 ?

@brettmillerb
Copy link
Contributor Author

@brettmillerb brettmillerb commented Nov 22, 2019

Made the recommendations by @TylerLeonhardt and renamed the parent folder to major version. Next version will be .NET core 3.0 and powershell 7 GA hopefully.

Let me know if there's anything else @Chuxel 🙂

@Chuxel
Copy link
Member

@Chuxel Chuxel commented Nov 23, 2019

Looks good - One quick question. Given .NET 2.2 is EOL Jan 1st 2020, should this use the LTS 2.1 SDK?

@TylerLeonhardt
Copy link
Member

@TylerLeonhardt TylerLeonhardt commented Nov 23, 2019

@Chuxel the PowerShell worker targets netcoreapp2.2 so I don't think we can do that.

Copy link
Member

@TylerLeonhardt TylerLeonhardt left a comment

LGTM

@Chuxel
Copy link
Member

@Chuxel Chuxel commented Nov 23, 2019

Thanks for the clarification! Merging.

@Chuxel Chuxel merged commit 20035cc into microsoft:master Nov 23, 2019
1 check passed
@bergmeister
Copy link
Contributor

@bergmeister bergmeister commented Jan 6, 2020

@TylerLeonhardt @brettmillerb Since the PowerShell worker uses .Net Core 2.1 (see here), 2.2 does not make any sense and has been deprecated in December. The template should've really targeted 2.1. I opened this PR: #183

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

Successfully merging this pull request may close these issues.

None yet

4 participants