Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-4710

host identification info needs to be part of log message

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: ctrl_events
    • Labels:
      None

      Description

      The EventAppender needs to add host identification (host/process/id) information to the log message it transmits. This was inadvertently left out.

        Attachments

          Activity

          Hide
          spietrowicz Steve Pietrowicz added a comment -

          Greg... This is a fairly simple ticket. Can you take a look at it?

          Show
          spietrowicz Steve Pietrowicz added a comment - Greg... This is a fairly simple ticket. Can you take a look at it?
          Hide
          gdaues Greg Daues added a comment -

          For starters, I can note that the scope of the ticket is actually a little more extensive that the title
          suggests (more work done that advertised!) in that host, process, and runid information are managed,
          and refactoring of classes was required to accomplish this.

          I also see these issues: for one, the handling of "ID", "Id" in naming looks like it can be made uniform.
          A new function 'setRunId' is added, but within the package there is already 'getLocalID',
          'getProcessID' and 'int localID'. I actually prefer 'setRunId', but I imagine we already have a standard
          to apply for this. Second, the use of a '_psp' for a PropertySet Ptr seemed curious. I looked through afw
          and observed that coders there have been consistent in naming the PropertySet Ptr's with
          "what they are", rather than just the type, examples: additionaData, metadata, fitsHdr, storage.
          The changeset has actually already taken steps along these lines, replacing "psp" with "logProp"
          in EventAppender.cc, and so I wonder if it is good to continue in this direction, with an alternate name
          for "_psp" if it can make sense in that context. Others than these small points, I consider
          the changes suitable and ready for merging.

          Show
          gdaues Greg Daues added a comment - For starters, I can note that the scope of the ticket is actually a little more extensive that the title suggests (more work done that advertised!) in that host, process, and runid information are managed, and refactoring of classes was required to accomplish this. I also see these issues: for one, the handling of "ID", "Id" in naming looks like it can be made uniform. A new function 'setRunId' is added, but within the package there is already 'getLocalID', 'getProcessID' and 'int localID'. I actually prefer 'setRunId', but I imagine we already have a standard to apply for this. Second, the use of a '_psp' for a PropertySet Ptr seemed curious. I looked through afw and observed that coders there have been consistent in naming the PropertySet Ptr's with "what they are", rather than just the type, examples: additionaData, metadata, fitsHdr, storage. The changeset has actually already taken steps along these lines, replacing "psp" with "logProp" in EventAppender.cc, and so I wonder if it is good to continue in this direction, with an alternate name for "_psp" if it can make sense in that context. Others than these small points, I consider the changes suitable and ready for merging.
          Hide
          spietrowicz Steve Pietrowicz added a comment -

          Thanks for the review. The "setRunId" in particular was named that to try an remain consistent with the existing "getRunId" method in the same class. The remaining Id vs ID and renaming of _psp to something better should probably done later in a refactor for another ticket, rather than expanding this one further.

          Show
          spietrowicz Steve Pietrowicz added a comment - Thanks for the review. The "setRunId" in particular was named that to try an remain consistent with the existing "getRunId" method in the same class. The remaining Id vs ID and renaming of _psp to something better should probably done later in a refactor for another ticket, rather than expanding this one further.

            People

            • Assignee:
              spietrowicz Steve Pietrowicz
              Reporter:
              spietrowicz Steve Pietrowicz
              Reviewers:
              Greg Daues
              Watchers:
              Greg Daues, Steve Pietrowicz
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Summary Panel