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

Refactor func metadata to allow multiple sources #2834

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

jviau
Copy link
Contributor

@jviau jviau commented Nov 6, 2024

Issue describing the changes in this PR

resolves #1159

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 -- TODO
  • My changes do not need to be backported to a previous version
    • Otherwise: Backport tracked by issue/PR #issue_or_pr
  • I have added all required tests (Unit tests, E2E tests) -- TODO

Additional information

NOTE: Tests are not updated yet. Will invest the effort on that when/if we accept this approach.

This PR refactors function metadata flow to allow for multiple sources. The general idea is now DefaultFunctionMetadataProvider is now registered as a primary provider, even with source gen. Source gen sets an internal options value which will change the behavior of DefaultFunctionMetadataProvider to skip scanning function.metadata, and only return results from IFunctionMetadataSource implementations. Source gen is now one of those sources.

Concretely, the changes are:

  1. Introduce IFunctionMetadataSource - a new extensibility contract that is expected to be 0 to N implementations by extensions, customers, etc. This allows for providing custom function metadata which is not the typical [Function(name)] flow. IE: durable can now give metadata to directly invoke class-based orchestrations and activities.
  2. Source gen now generates IFunctionMetadataSource along with IFunctionMetadataProvider (provider contract no longer used for this type).
  3. DefaultFunctionMetadataProvider is now updated to perform the following:
    • Unless disabled via options, will scan and provide metadata from functions.metadata file.
    • Aggregates results from all registered IFunctionMetadataSource.

Benefits

This approach offers a significant benefit of being compatible with newer versions of DotnetWorker.Core and old versions of Sdk.Generators. As in this case, we will still outright replace the implementation of IFunctionMetadataProvider with the source-gen implementation. This is a very real scenario customers could get into, and avoiding a subtle behavior regression here is important.

Concerns

  • Is changing the source gen emitted types contract a breaking change? -- added IFunctionMetadataProvider back to the emitted type, albeit it is no longer used.
  • Are the internal options on WorkerOptions how we want to go?
    • Really there are so many ways to approach this. What we need is some mechanism for source gen to disable the default behavior.
  • Alternatives to WorkerOptions.Internal:
    • Separate options object (InternalWorkerOptions?)
    • Attribute (if at least one IFunctionsMetadataSource is decorated with DefaultFunctionMetadataSource, default provider will skip scanning function.metadata)
    • Add a bool IsDefault to the IFunctionsMetadataSource (and switch to an abstract class instead of interface to default this to false).

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.

Allow for multiple IFunctionMetadataProvider implementations
1 participant