Skip to content

Added RetryContext #712

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Dec 3, 2021
Merged

Added RetryContext #712

merged 7 commits into from
Dec 3, 2021

Conversation

@michaelpeng36
Copy link
Contributor

Issue describing the changes in this PR

Resolves #594.

Pull request checklist

  • My changes do not require documentation changes
    • Otherwise: Documentation issue linked to PR
  • My changes should not be added to the release notes for the next release
    • Otherwise: I've added my notes to release_notes.md
  • I have added all required tests (Unit tests, E2E tests)

@michaelpeng36
Copy link
Contributor Author

I am planning on backporting these changes to the v4.x/ps7.0, v3.x/ps7, and v3.x/ps6. @Francisco-Gamino and @AnatoliB , let me know if backporting to these branches makes sense to you.

@Francisco-Gamino
Copy link
Contributor

I am planning on backporting these changes to the v4.x/ps7.0, v3.x/ps7, and v3.x/ps6. @Francisco-Gamino and @AnatoliB , let me know if backporting to these branches makes sense to you.

Thank you @michaelpeng36 for sending this out!

We should only port this change to v4.x/ps7.0, v3.x/ps7. Do you agree @AnatoliB?

@AnatoliB
Copy link
Contributor

AnatoliB commented Dec 2, 2021

We should only port this change to v4.x/ps7.0, v3.x/ps7. Do you agree @AnatoliB Anatoli Beliaev FTE?

I agree, no need to port to /ps6.

Francisco-Gamino
Francisco-Gamino requested changes Dec 2, 2021
View reviewed changes
release_notes.md
@@ -1 +1,2 @@
* Bug fix: Activity Functions can now use output bindings (https://github.com/Azure/azure-functions-powershell-worker/issues/646)
Copy link
Contributor

Choose a reason for hiding this comment

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

We do not need to update this file. Please remove it from the PR.

@Francisco-Gamino
Copy link
Contributor

Francisco-Gamino commented Dec 2, 2021 •
edited
Loading

@michaelpeng36 -- I left a couple of minor comments.

Adding the $RetryContext variable will required the templates to be updated. Could you please open an issue at https://github.com/Azure/azure-functions-templates/issues for this? Thanks!

/cc @stefanushinardi

@ghost ghost removed the Needs: Author Feedback label Dec 2, 2021
Francisco-Gamino
Francisco-Gamino approved these changes Dec 2, 2021
View reviewed changes
Copy link
Contributor

@Francisco-Gamino Francisco-Gamino left a comment

Choose a reason for hiding this comment

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

LGTM

@AnatoliB
Copy link
Contributor

AnatoliB commented Dec 3, 2021

Adding the $RetryContext variable will required the templates to be updated. Could you please open an issue at https://github.com/Azure/azure-functions-templates/issues for this? Thanks!

This is supposed to be an optional parameter though. I'm not sure we should be updating templates for this, as I don't expect the vast majority of users to rely on $RetryContext, so perhaps we should avoid adding unnecessary noise. How is this handled for Node.js functions?

@michaelpeng36
Copy link
Contributor Author

For Node.js functions, it does not appear that updating templates was necessary.

@michaelpeng36 michaelpeng36 merged commit 99e6f50 into dev Dec 3, 2021
michaelpeng36 added a commit that referenced this pull request Dec 3, 2021
@michaelpeng36
Added RetryContext (#712)
b8a2595 
* Updated subtree from https://github.com/azure/azure-functions-language-worker-protobuf. Tag: v1.5.1-protobuf. Commit: 2fc14be59bbd8bb9f3f6275247047675b83fb3ca

* Add RetryContext

* Added tests

* Added unit tests and removed E2E tests

* Fix RetryContext unit test

* Updated release_notes.md

* Addressing requested changes
@michaelpeng36 michaelpeng36 mentioned this pull request Dec 3, 2021
Merged
5 tasks
@michaelpeng36 michaelpeng36 deleted the michaelpeng/retry-context branch December 3, 2021 20:13
michaelpeng36 added a commit that referenced this pull request Dec 3, 2021
@michaelpeng36
Added RetryContext (#712)
e9fa8e4 
* Updated subtree from https://github.com/azure/azure-functions-language-worker-protobuf. Tag: v1.5.1-protobuf. Commit: 2fc14be59bbd8bb9f3f6275247047675b83fb3ca

* Add RetryContext

* Added tests

* Added unit tests and removed E2E tests

* Fix RetryContext unit test

* Updated release_notes.md

* Addressing requested changes
@michaelpeng36 michaelpeng36 mentioned this pull request Dec 3, 2021
Merged
7 tasks
michaelpeng36 added a commit that referenced this pull request Dec 3, 2021
@michaelpeng36
Added RetryContext (#712) (#713)
6497fd9 
* Updated subtree from https://github.com/azure/azure-functions-language-worker-protobuf. Tag: v1.5.1-protobuf. Commit: 2fc14be59bbd8bb9f3f6275247047675b83fb3ca

* Add RetryContext

* Added tests

* Added unit tests and removed E2E tests

* Fix RetryContext unit test

* Updated release_notes.md

* Addressing requested changes
michaelpeng36 added a commit that referenced this pull request Dec 3, 2021
@michaelpeng36
Added RetryContext (#712) (#714)
def9765 
* Updated subtree from https://github.com/azure/azure-functions-language-worker-protobuf. Tag: v1.5.1-protobuf. Commit: 2fc14be59bbd8bb9f3f6275247047675b83fb3ca

* Add RetryContext

* Added tests

* Added unit tests and removed E2E tests

* Fix RetryContext unit test

* Updated release_notes.md

* Addressing requested changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Francisco-Gamino Francisco-Gamino Francisco-Gamino approved these changes

@AnatoliB AnatoliB Awaiting requested review from AnatoliB

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add support for RetryContext

3 participants

@michaelpeng36 @Francisco-Gamino @AnatoliB