-
Notifications
You must be signed in to change notification settings - Fork 46
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
Fixed DictFile.append(), Glidein logging, and added logserver #467
Fixed DictFile.append(), Glidein logging, and added logserver #467
Conversation
af95000
to
83a5f5c
Compare
d8181ec
to
91cfd5f
Compare
append was used only for environment variables in the condor submit file and was causing a malformed submit file add_environment was already used at times and is a better method. append was fixed and is still there commented in case we'd like to use it for other attributes in the future
91cfd5f
to
2b33bc0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor comments require to be addressed.
3141a0d
to
fa3dbd6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good and can be merged!
e8196fd
to
327d3c2
Compare
@namrathaurs Sorry for the after-review changes. Testing the RPM installed version I noticed that there were some missing parts (only in my manual deployment) and I improved also the error checking. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor comments need to be addressed.
Added php and python example for a server receiving Glidein logs Fixed JWT generation and GLIDEIN_LOG_RECIPIENTS_FACTORY param for Factory logserver, improved also code and docstrings, and added documentation for logging and log server Refactored the logging utility to have all public functions starting with 'glog_' Added an example custom script, logging_test.sh, using Glidein logging Fixed mod_ssl dependency, fixed ReleaseManagerLib bugs for RPM building, and replaced the deprecated optparse with argparse Added the glideinwms-logserver RPM and adjusted ReleaseManagerLib for it Thanks to @namrathaurs for the review and the suggestion to remove all the example secrets TODO: changes to url_dirs.desc require factory upgrade. Should a reconfig be sufficient?
327d3c2
to
815a049
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes are ready for merging!
Fixed and disabled DictFile.append(), replaced by add_environment()
append was used only for environment variables in the condor submit file and was causing a malformed submit file.
add_environment was already used at times and is a better method. append was fixed and is still there commented in case we'd like to use it for other attributes in the future.