Skip to content
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

Clarify repy.py --logfile option help string #104

Open
aaaaalbert opened this issue Sep 10, 2015 · 5 comments
Open

Clarify repy.py --logfile option help string #104

aaaaalbert opened this issue Sep 10, 2015 · 5 comments
Labels

Comments

@aaaaalbert
Copy link
Collaborator

@vladimir-v-diaz notes that the help string to repy.py's --logfile option omits critical detail.

  • The "file name" supplied is really used as a prefix, and the logfile gets a .old or .new suffix to allow for log rotation.
  • The logfile is created in the current directory at the time of calling repy.py (I think). It's definitely not using what --cwd says; that option only sets the vessel working dir.
@Ashmita89
Copy link
Contributor

Help string for log file does not include the below information in detail.
For point a.:
ut_repyv2api_commandlineargs.py actively explains the point a in its comments:

# Call Repy. If repy.py mistakingly consumes the --logfile arg, it 
# will create a file named logfile_prefix + ".old" (or .new if .old 
# already existed).
logfile_prefix = "REPY_CALLARG_TEST"
repy_process = portable_popen.Popen([sys.executable, "repy.py", 
    "restrictions.default", program_name, "--logfile", logfile_prefix])

For point b.
repy.py sets the vessel working directory based on --cwd attribute

 #Set Current Working Directory
      if options.cwd:
    os.chdir(options.cwd)

However this is called after the logging attribute is dealt with in repy.py
In this a circular buffer of size =16 KB is used .

   # set up the circular log buffer...
   # Armon: Initialize the circular logger before starting the nanny
   if options.logfile:
    # time to set up the circular logger
    loggerfo = loggingrepy.circular_logger(options.logfile)
    # and redirect err and out there...
    sys.stdout = loggerfo
    sys.stderr = loggerfo
   else:
    #let's make it so that the output (via print) is always flushed
    sys.stdout = loggingrepy.flush_logger(sys.stdout)

Hence,the --cwd option is not used for the log file .

@vladimir-v-diaz
Copy link
Contributor

@Ashmita89
If you haven't done so, would you like to make a pull request for this issue (to ensure the help string is correct)? You have verified the expected behaviour.

The current incorrect help string was a source of frustration when I first tried to locate where Seattle stored his log files.

@aaaaalbert
Copy link
Collaborator Author

@Ashmita89, the unit test you are referring to doesn't test how repy.py utilizes the --logfile option. Instead, it checks that options to repy.py and options to the sandboxed Repy program are indeed handled separately, https://github.com/SeattleTestbed/repy_v2/blob/master/testsV2/ut_repyv2api_commandlineargs.py#L4-L8

In the unit test, the --logfile option is a deliberate attempt to fool repy.py into consuming an argument that is meant for the sandboxed program. It would be dangerous if repy.py consumed that arg, because the user of the sandbox can control it and set it to whatever value.

@Ashmita89
Copy link
Contributor

The unit test log is only referred for understanding the filename created as the comment there gives a more detailed description. It can also be seen in repy.py logfile option uses a circular logger which is more explained in 'loggingrepy_core'
// filenames we'll use for the log data

    self.filenameprefix = fnp

    self.oldfn = fnp+".old"

    self.newfn = fnp+".new"

I think the information from 'loggingrepy_core' would have caused less confusion. We can ignore the part about the unit test file in my previous comment.

@aaaaalbert
Copy link
Collaborator Author

OK, please propose a help string that conveys this information, preferrentially via pull request.

Ashmita89 added a commit to Ashmita89/repy_v2 that referenced this issue Dec 15, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants