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

Add Ballerina language support #3584

Merged
merged 12 commits into from
Jul 14, 2023
Merged

Conversation

xlight05
Copy link
Contributor

@xlight05 xlight05 commented Feb 22, 2023

$Subject as discussed on #3361

Ballerina is a programming language designed for writing network applications. Ballerina has the support for azure functions using ballerinax/azure_functions package. This package gives users cleaner abstractions to write azure functions just like main languages(such as java), hiding away the complexity of using custom handlers directly. The programming model does not require users to write functions.json files as its automatically generated from the ballerina compiler itself. (Similar to node,python new programming model).

I've attached some screen recordings to cover some scenarios just to make the review process easier.

  1. Creating Project
tool.create-project.mp4
  1. Add function to existing project
tool.add-function-existing.mp4
  1. Initialize vscode in an existing project
tool.init-vscode-existing.mp4

Let me know if theres any screen recordings needs to be added.
Please review the PR carefully as this is my first time contributing to this repository :) Any suggestions, requests are welcome.

@xlight05 xlight05 requested a review from a team as a code owner February 22, 2023 13:37
@xlight05
Copy link
Contributor Author

@microsoft-github-policy-service agree company="Wso2"

@xlight05
Copy link
Contributor Author

xlight05 commented Mar 5, 2023

@nturinski Can you take a look at this PR when you have time please?

@xlight05 xlight05 changed the title [WIP] Add Ballerina language support Add Ballerina language support Mar 5, 2023
@alexweininger
Copy link
Member

alexweininger commented Mar 31, 2023

Where did you find the Ballerina templates? Or did you write those yourself?

edit: also thanks for your contribution!

@xlight05
Copy link
Contributor Author

xlight05 commented Apr 1, 2023

Where did you find the Ballerina templates? Or did you write those yourself?

edit: also thanks for your contribution!

Thankss. Yes, I wrote them myself. However, the plan is to make the templates similar(at least consistent) with ballerina docs as well.
https://ballerina.io/learn/run-in-the-cloud/function-as-a-service/azure-functions/
https://ballerina.io/learn/by-example/azure-functions-deployment/ (we'll be introducing separate examples like this for each trigger, this will align with the templates)

Copy link
Member

@nturinski nturinski left a comment

Choose a reason for hiding this comment

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

Thanks again for your contribution and your patience! I left a few comments, but also had some broader questions:

  • Would you be a good contact point if anything Ballerina ends up having issues? Most likely this will be users having issues debugging their project or fuddling with their configurations.
  • Would you be willing to add tests? I can assist you with that, but it would be just the create project tests with this specific runtime.

@xlight05
Copy link
Contributor Author

Thanks again for your contribution and your patience! I left a few comments, but also had some broader questions:

  • Would you be a good contact point if anything Ballerina ends up having issues? Most likely this will be users having issues debugging their project or fuddling with their configurations.
  • Would you be willing to add tests? I can assist you with that, but it would be just the create project tests with this specific runtime.

Thanks for reviewing, I'll reply and work on these suggestions.

  1. For any ballerina azure functions issue, you can contact me or @anuruddhal. You can link our discord as well. All of the ballerina team members are present there and they will provide answers within a few hours =)

  2. Yes I can definitely add tests. I found some test files related to javascript and c# but for java it was very minimal.I was not exactly sure about the test structure. If you could tell me the required files to add tests(and maybe provide a reference), I can take it from there.

I'll address other comments inline.

@xlight05
Copy link
Contributor Author

xlight05 commented May 8, 2023

@nturinski
I've added a commit with fixes for most of the comments and few more changes except the tests. This contains few more changes that weren't requested as well.

  • Removed prompt for package name, org, and version as they are redundant. Now, upon project creation, it executes bal init in the directory which initializes a ballerina package(Ballerina.toml) in the directory with guessed package name and orgs. This removes the need to writing package name guessing logic in the vscode side.

  • Added partial debug capability. We need to figure out the workerArgKey for custom handlers. I noticed in java, you can set this languageWorkers__java__arguments property as application properties and it gets picked out when the function is starting out by the func command. but for custom handlers that doesn't seem to be the case. :/. There maybe a different key for custom handlers.If you know how to do this/know anyone who does, please let me know. Made an issue as well to get this information. How to set custom handler arguments via Application settings? Azure/azure-functions-host#9260

    However, in host.json custom handler args you can just put the debug value and then the debug server starts up.
    https://gist.github.com/xlight05/3d4e58e2d78dda827cbfb5a1c3a62348
    Is it possible to add arguments to host.json using vscode ballerina debug provider?

I resolved the comments that have straightforward fixes. I left comments on all the other comments. please check =)

@xlight05
Copy link
Contributor Author

@nturinski Hi, Sorry, was figuring out a way to add args to startup for custom handler for providing debugger capabilities. Found a workaround for this. I'll send commit along with tests early next week. =) hope thats oky

Copy link
Member

@nturinski nturinski left a comment

Choose a reason for hiding this comment

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

Thanks for your patience and effort! While waiting for the workaround, I've left smaller comments and nits. Regarding adding args, is it possible to just add these to the func start command as flags and parameters? I'm not entirely sure what exactly you are trying to add.

src/constants.ts Outdated Show resolved Hide resolved
src/debug/BallerinaDebugProvider.ts Outdated Show resolved Hide resolved
src/templates/script/getScriptVerifiedTemplateIds.ts Outdated Show resolved Hide resolved
@xlight05
Copy link
Contributor Author

xlight05 commented Jun 5, 2023

@nturinski
I added some tests with e33eec5. Tests seem to be passing in my local pc. Seems like theres some other issues as the daily build also is failing. Can you review the tests and see if they are oky?

Also, ballerina does not have a reusable task for azure devops pipelines yet, I installed ballerina manually on test workflow as a workaround, we'll make sure to implement a proper task and use it here =)

Also kind reminder on #3584 (comment)

Debugger stuff seem to be oky, works fine =)

@nturinski
Copy link
Member

Also, ballerina does not have a reusable task for azure devops pipelines yet, I installed ballerina manually on test workflow as a workaround, we'll make sure to implement a proper task and use it here =)

That would be great! I had a few more comments but everything is looking good :)

@xlight05
Copy link
Contributor Author

Also, ballerina does not have a reusable task for azure devops pipelines yet, I installed ballerina manually on test workflow as a workaround, we'll make sure to implement a proper task and use it here =)

That would be great! I had a few more comments but everything is looking good :)

aye, will add changes during the weekend.
I noticed daily build was failing for long time. will we be able to fix the issue?

@nturinski
Copy link
Member

We're currently trying to fix them, but unfortunately there's been so many changes with the extension, it's been hard to keep everything working.

Could you pull down from main? I tried to do it for you, but I wasn't able to pull down your branch. I think it may have been too long since there was a push?

@xlight05
Copy link
Contributor Author

xlight05 commented Jul 10, 2023

We're currently trying to fix them, but unfortunately there's been so many changes with the extension, it's been hard to keep everything working.

Could you pull down from main? I tried to do it for you, but I wasn't able to pull down your branch. I think it may have been too long since there was a push?

I pushed a merge commit but daily build was failing that day too ;( however, seems like the daily build is passing today. I will push another commit to trigger the pr checks again.

I've added the ballerina template type as well to move away from the script provider. Please go through the code one more time to see if its correct.

Besides the azure pipelines task, I think we are almost done with the implementation right? Just FYI, I've wrote a sample azure pipelines task so that we can use it for this repo workflows.
We are planning to host the code in https://github.com/ballerina-platform/ballerina-azure-pipelines-task
Im in the process of sending that PR, publishing it to vscode marketplace and getting it verified.

This is exciting. Thanks a lot for helping through this process of getting the PR merged = )

Edit -: Seems like the pr checks are still failing ;( Are. these intermittent issues(as they are timeouts)? if so could you retrigger and see? I don't have permission to retrigger ;(

@nturinski
Copy link
Member

Seems like the pr checks are still failing ;( Are. these intermittent issues(as they are timeouts)? if so could you retrigger and see? I don't have permission to retrigger ;(

I kicked it off again, but honestly, since the other OS's are fine, I'd be fine to ignore the Windows results. We've been having issues with it so it's not related to any of your changes.

I'll finish up my review and we should be good to merge :) Thanks again for all your patience and collaboration!

Copy link
Member

@nturinski nturinski left a comment

Choose a reason for hiding this comment

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

Looks good to me! We'll merge this soon. Thanks again for all of your patience with our collaboration! I know that I had a lot of requests. We really appreciate your contribution :)

@xlight05
Copy link
Contributor Author

xlight05 commented Jul 12, 2023

Looks good to me! We'll merge this soon. Thanks again for all of your patience with our collaboration! I know that I had a lot of requests. We really appreciate your contribution :)

Great to hear! Thanks a lot for helping me throughout the process as well = )
Also, I finished the initial implementation of the DevOps pipeline task(You will still need to subscribe to the extension) -
VS marketplace - https://marketplace.visualstudio.com/items?itemName=ballerina.ballerina-devops-pipelines
Code - ballerina-platform/ballerina-azure-pipelines-task#1
Consumer - https://github.com/xlight05/az_pipelines_playground/blob/master/build.yml

If we can subscribe to this we can get rid of ballerina installers in the workflows :D

@nturinski nturinski merged commit cd2e632 into microsoft:main Jul 14, 2023
@microsoft microsoft locked and limited conversation to collaborators Aug 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants