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

SDK should Honor FunctionName attribute #1170

Closed
MikeStall opened this issue Jun 2, 2017 · 8 comments
Closed

SDK should Honor FunctionName attribute #1170

MikeStall opened this issue Jun 2, 2017 · 8 comments
Assignees
Milestone

Comments

@MikeStall
Copy link
Contributor

MikeStall commented Jun 2, 2017

We added a FunctionName attribute #1075
It's only a placeholder used by VS tooling, but SDK should honor this attribute too and plumb it through.

  1. Function indexing
  2. {sys.MethodName}
  3. JobHost.Call? (maybe moot since that uses MethodInfo, but double check)
  4. Dashboard
  5. New InvokeFunctionFilterAttribute attribute that specifies function names
@paulbatum paulbatum added this to the Next milestone Jun 5, 2017
@paulbatum
Copy link
Member

sys.MethodName should be the actual name of the method, it shouldn't be overridden by the functionname attribute

@MikeStall
Copy link
Contributor Author

@paulbatum - I'd disagree. The actual C# method name should be viewed as an implementation detail - we don't want things taking a direct dependency on that. VS Tooling includes [FunctionName] attributes by default and that's what they show up in Script. For example, Bot uses {sys.MethodName} and expects to get the Script function name which would be [FunctionName].

@fabiocav
Copy link
Member

fabiocav commented Jun 5, 2017

I agree that {sys.MethodName} should reflect the method name as opposed to the logical function name. If we want to expose a way to get the function name, we should probably introduce a {sys.FunctionName} instead, as passing the function name when binding to the method would be surprising/confusing.

@MikeStall
Copy link
Contributor Author

@fabiocav - please see my comments above. Exposing the reflection name would actually be incorrect; and there's not a good reason to add that low-level functionality.

@fabiocav
Copy link
Member

fabiocav commented Jun 5, 2017

I agree, but I'd argue that, given its name today, that's what people would expect. We don't refer to the function name as a method name anywhere. Introducing a FunctionName and perhaps deprecating MethodName would probably be the right approach.

@MikeStall
Copy link
Contributor Author

This is recently new surface area. If it's a naming issue, we can do a full rename MethodName --> FunctionName, and we just need to update BOT. But we shouldn't have anything handing out the underlying Reflection name.

@paulbatum
Copy link
Member

I'm fine with that, assuming we never disclosed sys.methodname publicly and therefore not a breaking change

@MikeStall
Copy link
Contributor Author

MikeStall commented Jun 5, 2017

It was added here: 962dfa6 on April 24th.

@MikeStall MikeStall modified the milestones: Sprint 2, Next Jul 12, 2017
@MikeStall MikeStall self-assigned this Jul 12, 2017
MikeStall added a commit that referenced this issue Jul 14, 2017
…criptor.

Remove FunctionDescriptor.MethodInfo. Adding new fields on FunctionDescriptor like LogName, IsDisabled, HasCanncelationtoken.
  methodInfo.DeclaringType.Name + "." +methodInfo.Name --> FunctionDescriptor.ShortName
  methodInfo.Name --> FunctionDescriptor.LogName

Continuation of work started in
183e3c4

No functional change here. This is prepping for the [FunctionName] support. (#1170)
MikeStall added a commit that referenced this issue Jul 14, 2017
…criptor.

Remove FunctionDescriptor.MethodInfo. Adding new fields on FunctionDescriptor like LogName, IsDisabled, HasCanncelationtoken.
  methodInfo.DeclaringType.Name + "." +methodInfo.Name --> FunctionDescriptor.ShortName
  methodInfo.Name --> FunctionDescriptor.LogName

Continuation of work started in
183e3c4

No functional change here. This is prepping for the [FunctionName] support. (#1170)
MikeStall added a commit that referenced this issue Jul 14, 2017
…criptor.

Remove FunctionDescriptor.MethodInfo. Adding new fields on FunctionDescriptor like LogName, IsDisabled, HasCanncelationtoken.
  methodInfo.DeclaringType.Name + "." +methodInfo.Name --> FunctionDescriptor.ShortName
  methodInfo.Name --> FunctionDescriptor.LogName

Continuation of work started in
183e3c4

No functional change here. This is prepping for the [FunctionName] support. (#1170)
MikeStall added a commit that referenced this issue Jul 18, 2017
SDK honors [FunctionName] attribute

ADd new JobHost.CallAsync(string) endpoint which can honor the method name.
MikeStall added a commit that referenced this issue Jul 18, 2017
SDK honors [FunctionName] attribute

ADd new JobHost.CallAsync(string) endpoint which can honor the method name.
MikeStall added a commit that referenced this issue Jul 19, 2017
Resolve #1170

Add new JobHost.CallAsync(string) endpoint which can honor the method name.
MikeStall added a commit that referenced this issue Jul 19, 2017
Resolve #1170

Add new JobHost.CallAsync(string) endpoint which can honor the method name.
MikeStall added a commit that referenced this issue Jul 19, 2017
…criptor.

Remove FunctionDescriptor.MethodInfo. Adding new fields on FunctionDescriptor like LogName, IsDisabled, HasCanncelationtoken.
  methodInfo.DeclaringType.Name + "." +methodInfo.Name --> FunctionDescriptor.ShortName
  methodInfo.Name --> FunctionDescriptor.LogName

Continuation of work started in
183e3c4

No functional change here. This is prepping for the [FunctionName] support. (#1170)

# Conflicts:
#	test/Microsoft.Azure.WebJobs.Host.UnitTests/Singleton/SingletonListenerTests.cs
#	test/Microsoft.Azure.WebJobs.Host.UnitTests/WebJobs.Host.UnitTests.csproj
MikeStall added a commit that referenced this issue Jul 19, 2017
Resolve #1170

Add new JobHost.CallAsync(string) endpoint which can honor the method name.
hhawkins pushed a commit to hhawkins/azure-webjobs-sdk that referenced this issue Jul 19, 2017
Resolve Azure#1170

Add new JobHost.CallAsync(string) endpoint which can honor the method name.
MikeStall added a commit that referenced this issue Jul 20, 2017
…criptor.

Remove FunctionDescriptor.MethodInfo. Adding new fields on FunctionDescriptor like LogName, IsDisabled, HasCanncelationtoken.
  methodInfo.DeclaringType.Name + "." +methodInfo.Name --> FunctionDescriptor.ShortName
  methodInfo.Name --> FunctionDescriptor.LogName

Continuation of work started in
183e3c4

No functional change here. This is prepping for the [FunctionName] support. (#1170)

# Conflicts:
#	test/Microsoft.Azure.WebJobs.Host.UnitTests/Singleton/SingletonListenerTests.cs
#	test/Microsoft.Azure.WebJobs.Host.UnitTests/WebJobs.Host.UnitTests.csproj
MikeStall added a commit that referenced this issue Jul 20, 2017
Resolve #1170

Add new JobHost.CallAsync(string) endpoint which can honor the method name.
alrod pushed a commit to alrod/azure-functions-servicebus-extension that referenced this issue Jul 3, 2019
…criptor.

Remove FunctionDescriptor.MethodInfo. Adding new fields on FunctionDescriptor like LogName, IsDisabled, HasCanncelationtoken.
  methodInfo.DeclaringType.Name + "." +methodInfo.Name --> FunctionDescriptor.ShortName
  methodInfo.Name --> FunctionDescriptor.LogName

Continuation of work started in
Azure/azure-webjobs-sdk@183e3c4

No functional change here. This is prepping for the [FunctionName] support. (Azure/azure-webjobs-sdk#1170)
alrod pushed a commit to alrod/azure-functions-servicebus-extension that referenced this issue Jul 3, 2019
Resolve Azure/azure-webjobs-sdk#1170

Add new JobHost.CallAsync(string) endpoint which can honor the method name.
alrod pushed a commit to alrod/azure-functions-servicebus-extension that referenced this issue Jul 5, 2019
…criptor.

Remove FunctionDescriptor.MethodInfo. Adding new fields on FunctionDescriptor like LogName, IsDisabled, HasCanncelationtoken.
  methodInfo.DeclaringType.Name + "." +methodInfo.Name --> FunctionDescriptor.ShortName
  methodInfo.Name --> FunctionDescriptor.LogName

Continuation of work started in
Azure/azure-webjobs-sdk@183e3c4

No functional change here. This is prepping for the [FunctionName] support. (Azure/azure-webjobs-sdk#1170)
alrod pushed a commit to alrod/azure-functions-servicebus-extension that referenced this issue Jul 5, 2019
Resolve Azure/azure-webjobs-sdk#1170

Add new JobHost.CallAsync(string) endpoint which can honor the method name.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants