-
Notifications
You must be signed in to change notification settings - Fork 204
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
Adding support to provide start date time for cosmos feed start #691
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@brettsam Do we have any other trigger that passes datetime information? What is the recommendation on the format?
src/WebJobs.Extensions.CosmosDB/Trigger/CosmosDBTriggerAttribute.cs
Outdated
Show resolved
Hide resolved
src/WebJobs.Extensions.CosmosDB/Trigger/CosmosDBTriggerAttribute.cs
Outdated
Show resolved
Hide resolved
src/WebJobs.Extensions.CosmosDB/Trigger/CosmosDBTriggerAttributeBindingProvider.cs
Outdated
Show resolved
Hide resolved
test/WebJobs.Extensions.CosmosDB.Tests/Trigger/CosmosDBTriggerAttributeBindingProviderTests.cs
Outdated
Show resolved
Hide resolved
test/WebJobs.Extensions.CosmosDB.Tests/Trigger/CosmosDBTriggerAttributeBindingProviderTests.cs
Outdated
Show resolved
Hide resolved
src/WebJobs.Extensions.CosmosDB/Trigger/CosmosDBTriggerAttributeBindingProvider.cs
Outdated
Show resolved
Hide resolved
src/WebJobs.Extensions.CosmosDB/Trigger/CosmosDBTriggerAttributeBindingProvider.cs
Outdated
Show resolved
Hide resolved
A gentle ping on the review! |
src/WebJobs.Extensions.CosmosDB/Trigger/CosmosDBTriggerAttribute.cs
Outdated
Show resolved
Hide resolved
src/WebJobs.Extensions.CosmosDB/Trigger/CosmosDBTriggerAttribute.cs
Outdated
Show resolved
Hide resolved
src/WebJobs.Extensions.CosmosDB/Trigger/CosmosDBTriggerAttributeBindingProvider.cs
Outdated
Show resolved
Hide resolved
src/WebJobs.Extensions.CosmosDB/Trigger/CosmosDBTriggerAttributeBindingProvider.cs
Outdated
Show resolved
Hide resolved
src/WebJobs.Extensions.CosmosDB/Trigger/CosmosDBTriggerAttributeBindingProvider.cs
Outdated
Show resolved
Hide resolved
src/WebJobs.Extensions.CosmosDB/Trigger/CosmosDBTriggerAttributeBindingProvider.cs
Outdated
Show resolved
Hide resolved
src/WebJobs.Extensions.CosmosDB/Trigger/CosmosDBTriggerAttributeBindingProvider.cs
Outdated
Show resolved
Hide resolved
test/WebJobs.Extensions.CosmosDB.Tests/Trigger/CosmosDBTriggerAttributeBindingProviderTests.cs
Show resolved
Hide resolved
…te.cs Co-authored-by: Matias Quaranta <ealsur@users.noreply.github.com>
…teBindingProvider.cs Co-authored-by: Matias Quaranta <ealsur@users.noreply.github.com>
…teBindingProvider.cs Co-authored-by: Matias Quaranta <ealsur@users.noreply.github.com>
test/WebJobs.Extensions.CosmosDB.Tests/Trigger/CosmosDBTriggerAttributeBindingProviderTests.cs
Show resolved
Hide resolved
src/WebJobs.Extensions.CosmosDB/Trigger/CosmosDBTriggerAttributeBindingProvider.cs
Show resolved
Hide resolved
I don't think we do -- I was just trying to think through that as well. I know we have some that use TimeSpan, let me double-check how we handle that. |
My concern is more around other languages, if the ISO format would work fine for them or there is a different one. Otherwise I see no blockers on the PR other than the test gap at this point. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have one other comment that I can't directly link to (and maybe it can be an immediate follow-up PR by @ealsur?) -- but now that the parsing of attribute strings can have some interesting side effects (time zones...), it may be good to explicitly log the fully-resolved ProcessorOptions
with a _logger.LogDebug(...)
statement here: https://github.com/Azure/azure-webjobs-sdk-extensions/blob/dev/src/WebJobs.Extensions.CosmosDB/Trigger/CosmosDBTriggerListener.cs#L129. That would help immensely when trying to support folks that may have configured something slightly incorrectly.
src/WebJobs.Extensions.CosmosDB/Trigger/CosmosDBTriggerAttributeBindingProvider.cs
Show resolved
Hide resolved
Deserialization of chunked requests does not work (Issue Azure#702)
Exposing StartTime of ChangeFeedProcessorOptions to client attribute so that a client to specify the start point of the feed. The date time should ideally be in UTC but it can take any format that can be parsed by C# DateTime
Added default test case to check the added attribute.