Print

Print


@abh3 requested changes on this pull request.

Looks OK. Two things: 1) possible mislabeling a message, and 2) I'd prefer you use the new way of compactly handling the config file so we don't have code splatter.


In src/XrdSciTokens/XrdSciTokensAccess.cc:

>              } catch (std::exception &exc) {
-                m_log.Emsg("Access", "Error generating ACLs for authorization", exc.what());
+                m_log.Log(LogMask::Info, "Access", "Error generating ACLs for authorization", exc.what());

Why is this message being relegated to info? It seems hat one would want to know this has happened, yes?


In src/XrdSciTokens/XrdSciTokensAccess.cc:

> @@ -716,34 +802,77 @@ class XrdAccSciTokens : public XrdAccAuthorize, public XrdSciTokensHelper
         return true;
     }
 
+
+    bool Config() {

I think you should replace all of this with the new way of getting the plug-in config. Look at XrdOucGatherConf.hh which pretty much replaces all the manual handling of the config file. I'd prefer to start using this for all new code.


In src/XrdSciTokens/XrdSciTokensAccess.cc:

> @@ -1,6 +1,7 @@
 
 #include "XrdAcc/XrdAccAuthorize.hh"
 #include "XrdOuc/XrdOucEnv.hh"
+#include "XrdOuc/XrdOucStream.hh"

If you use XrdOucGatherConf.hh then there is no need to include this file. See detailed notes in this at "Config()".


Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.Message ID: <xrootd/xrootd/pull/1644/review/910954448@github.com>

[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/xrootd/xrootd/pull/1644#pullrequestreview-910954448", "url": "https://github.com/xrootd/xrootd/pull/1644#pullrequestreview-910954448", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]

Use REPLY-ALL to reply to list

To unsubscribe from the XROOTD-DEV list, click the following link:
https://listserv.slac.stanford.edu/cgi-bin/wa?SUBED1=XROOTD-DEV&A=1