>From Murtadha Hubail <[email protected]>: Attention is currently required from: Michael Blow, Hussain Towaileb. Murtadha Hubail has posted comments on this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19344 )
Change subject: [ASTERIXDB-3514][EXT]: Add error codes for missing invalid/missing creds to assume role ...................................................................... Patch Set 7: (1 comment) File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/external/IExternalCredentialsCache.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19344/comment/667f4882_2f114b02 PS7, Line 74: getName(Map<String, String> configuration) throws CompilationException; : : /** : * Returns the name of the entity which the cached credentials belong to : * : * @param fqn fully qualified name for the credentials entity : * @return name of entity which credentials belong to : */ : String getName(IFullyQualifiedName fqn) throws CompilationException; These two APIs don't belong in this interface. Think about it this way: I'm an ExternalCredentailsCache, why do I have an API that returns a name? It shouldn't be my responsibility to give you names. We also shouldn't have those duplicate APIs with different signatures. Also, the logic to construct fully qualified names shouldn't be in the implementation of this class or in multiple code locations. The name to be used for caching, updating, or dropping the cache should either be: 1) Passed to the APIs 2) Already be in the configuration with a clear way to access it (using a constant key) 3) The entity passed in the API has a getter for the name (e.g., IExternalEntity#getName). I know the current code paths don't make it easy to do that and it will require some refactoring, but the current readability of ExternalCredentialsCache isn't great. If you prefer, we can merge this patch for the other fixes, then we can discuss the way to fix the APIs. Let me know your preference. -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19344 To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Change-Id: I3ea48cc83d4c0b92d66e518f7e8108050f0e553a Gerrit-Change-Number: 19344 Gerrit-PatchSet: 7 Gerrit-Owner: Hussain Towaileb <[email protected]> Gerrit-Reviewer: Hussain Towaileb <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Michael Blow <[email protected]> Gerrit-Reviewer: Murtadha Hubail <[email protected]> Gerrit-Attention: Michael Blow <[email protected]> Gerrit-Attention: Hussain Towaileb <[email protected]> Gerrit-Comment-Date: Fri, 24 Jan 2025 12:09:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
