@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.
> } 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?
> @@ -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.
> @@ -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 or view it on GitHub:
https://github.com/xrootd/xrootd/pull/1644#pullrequestreview-910954448
You are receiving this because you are subscribed to this thread.
Message ID: <[log in to unmask]>
########################################################################
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
