Creation of DTS example and passing of completionToken #40
Conversation
This file contains hidden or 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
Signed-off-by: Ryan Lettieri <67934986+RyanLettieri@users.noreply.github.com>
cgillum
reviewed
Jan 22, 2025
Member
cgillum
left a comment
cgillum
left a comment
There was a problem hiding this comment.
The code changes look good, though I have feedback on the sample and README.
Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com>
Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com>
berndverst
approved these changes
Jan 22, 2025
torosent
requested changes
Jan 23, 2025
Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com>
Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com>
Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com>
cgillum
requested changes
Jan 27, 2025
Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com>
RyanLettieri
force-pushed
the
durabletask-scheduler
branch
from
05af892 to
6050771
Compare
Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com>
Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com>
torosent
reviewed
Jan 29, 2025
cgillum
reviewed
Jan 29, 2025
Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com>
Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com>
Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com>
Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com>
cgillum
requested changes
Feb 7, 2025
Member
cgillum
left a comment
cgillum
left a comment
There was a problem hiding this comment.
Haven't finished reviewing the whole thing yet, but have some fairly significant feedback for what I've reviewed so far.
Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com>
Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com>
Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com>
cgillum
requested changes
Feb 11, 2025
Member
cgillum
left a comment
cgillum
left a comment
There was a problem hiding this comment.
The updated design is looking great. A few technical things that we still need to clean up. I haven't reviewed everything quite yet, though.
Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com>
Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com>
cgillum
reviewed
Feb 12, 2025
cgillum
reviewed
Feb 12, 2025
Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com>
Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com>
Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com>
cgillum
reviewed
Feb 13, 2025
Member
cgillum
left a comment
cgillum
left a comment
There was a problem hiding this comment.
I think we're super close. A few more things to clean up.
durabletask-azuremanaged/durabletask/azuremanaged/internal/access_token_manager.py
Outdated
Show resolved
Hide resolved
Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com>
cgillum
reviewed
Feb 13, 2025
Member
cgillum
left a comment
cgillum
left a comment
There was a problem hiding this comment.
Looks like the tests are failing, but just a couple more things in addition to that.
durabletask-azuremanaged/durabletask/azuremanaged/durabletask_grpc_interceptor.py
Outdated
Show resolved
Hide resolved
Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com>
cgillum
reviewed
Feb 13, 2025
cgillum
reviewed
Feb 13, 2025
Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com>
Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com>
Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com>
cgillum
reviewed
Feb 14, 2025
Member
cgillum
left a comment
cgillum
left a comment
There was a problem hiding this comment.
OK, changes look good. I'll sign off after doing some local validation. I may add another commit as well depending on how things go.
cgillum
approved these changes
Feb 14, 2025
Member
|
EDIT: Never mind - I see the pyproject is nested. All good there. But it feels weird to have a top level pyproject and then another pyproject somewhere in a nested folder. Maybe we could consider rearranging that structure in the future. |
berndverst
reviewed
Feb 14, 2025
berndverst
reviewed
Feb 14, 2025
berndverst
reviewed
Feb 14, 2025
berndverst
reviewed
Feb 14, 2025
berndverst
reviewed
Feb 14, 2025
Member
While this is not strictly necessary - it may be a good idea to use type hints throughout. For example here we have an attribute that has a datetime but also None assigned to it. I ran For now since you don't have any typing at other than the method signature (and neither does |
Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com>
Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com>
Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com>
berndverst
approved these changes
Feb 18, 2025
acroca
pushed a commit
to acroca/dapr-durabletask-python
that referenced
this pull request
Jun 17, 2025
* Creation of DTS example and passing of completionToken Signed-off-by: Ryan Lettieri <67934986+RyanLettieri@users.noreply.github.com> * Adressing review feedback Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com> * Reverting dapr readme Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com> * Adding accessTokenManager class for refreshing credential token Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com> * Adding comments to the example Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com> * Adding in requirement for azure-identity Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com> * Moving dts logic into its own module Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com> * Fixing whitesapce Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com> * Updating dts client to refresh token Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com> * Cleaning up construction of dts objects and improving examples Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com> * Migrating shared access token logic to new grpc class Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com> * Adding log statements to access_token_manager Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com> * breaking for loop when setting interceptors Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com> * Removing changes to client.py and adding additional steps to readme.md Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com> * Refactoring client and worker to pass around interceptors Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com> * Fixing import for DefaultClientInterceptorImpl Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com> * Adressing round 1 of feedback Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com> * Fixing interceptor issue Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com> * Moving some files around to remove dependencies Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com> * Adressing more feedback Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com> * More review feedback Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com> * Passing token credential as an argument rather than 2 strings Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com> * More review feedback for token passing Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com> * Addressing None comment and using correct metadata Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com> * Updating unit tests Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com> * Fixing the type for the unit test Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com> * Fixing grpc calls Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com> * Fix linter errors and update documentation * Specifying version reqiuirement for pyproject.toml Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com> * Updating README Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com> * Adding comment for credential type Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com> --------- Signed-off-by: Ryan Lettieri <67934986+RyanLettieri@users.noreply.github.com> Signed-off-by: Ryan Lettieri <ryanLettieri@microsoft.com> Co-authored-by: Chris Gillum <cgillum@microsoft.com> Signed-off-by: Albert Callarisa <albert@diagrid.io>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
This PR adds in the completion token to the worker.py class for DTS to understand when a task or work item is complete.
This PR also adds in an example of using the DTS backend and updates the README to reflect the necessary steps