Support Oauth 2.0 Authentication for Event Grid API #855
Conversation
Cherry pick to beta success |
sjuarezgx
had a problem deploying
to
external-storage-tests
GitHub Actions
Failure
…re not being executed.
sjuarezgx
had a problem deploying
to
external-storage-tests
GitHub Actions
Failure
Cherry pick to beta success |
| else | ||
| //Try authenticating using AD | ||
| { | ||
| logger.Debug("Authentication using Oauth 2.0."); |
There was a problem hiding this comment.
Would it be posible to use GXLogging instead of the direct logger? This way, we can reuse the logic. For instance, GXLogging checks for enabled logs in each method.
The project already has a reference to GXEventRouter which references GxClasses.
| else | ||
| { | ||
| throw ae; | ||
| throw new Exception("There was an error at the Event Grid initialization."); |
There was a problem hiding this comment.
Throwing exceptions is more expensive than setting an error code or returning a message. Have we already considered managing errors in a way other than throwing exceptions?
There was a problem hiding this comment.
It seemed like a good idea to me as it is now (I saw it in other projects) because the implementation of the methods for each provider was cleaner, and then in the class that implements the EO handles the errors...
Besides, it cannot be changed for some methods like Connect, because it calls using reflection the provider´s implementation and catches the exceptions..
sjuarezgx
temporarily deployed
to
external-storage-tests
GitHub Actions
Inactive
Cherry pick to beta success |
claudiamurialdo
left a comment
claudiamurialdo
left a comment
There was a problem hiding this comment.
Except for the comment about avoiding to create and throw exceptions, the rest LGTM.
Issue:104035