Skip to content

Minor adjustments for imagej-omero#2173

Merged
joshmoore merged 4 commits intoome:dev_5_0from
joshmoore:imagej-omero-0.1.0
Apr 7, 2014
Merged

Minor adjustments for imagej-omero#2173
joshmoore merged 4 commits intoome:dev_5_0from
joshmoore:imagej-omero-0.1.0

Conversation

@joshmoore
Copy link
Copy Markdown
Member

Various small fixes and improvements found while working on https://github.com/imagej/imagej-omero/issues?milestone=3&state=open with @ctrueden.

For review:

  • @sbesson et al: thoughts on the addition of bin/omero admin kill?
  • Addition of OMERO_HOME should have no impact on scripts. (In fact, it can likely be considered part of the public API) /cc @will-moore
  • Similarly, bin/omero script params /test.py should now print out both stdout and stderr to help in debugging (See 'Can't find params' should list stdout file ID imagej/imagej-omero#18)
  • @ximenesuk: the filtering for org.bushe likely isn't needed, but as we will need to consider which logback configuration we're going to pass to background Java tasks.

Happy to break this up if need be, but I wanted to get it open for discussion.

@will-moore
Copy link
Copy Markdown
Member

I'm not seeing 'OMERO_HOME' available for scripts using os.environ['OMERO_HOME']
on trout:

$ omero script launch 19701
Using session 5e409098-6bdc-4ad9-870d-a7fb3bd78484 (root@trout.openmicroscopy.org:4064). Idle timeout: 10.0 min. Current group: system
Job 7454 ready
Waiting....
Callback received: FINISHED

    *** start stdout (id=19706)***
    * OMERO_HOME
    * 
    *** end stdout ***


    *** start stderr (id=19707)***
    * Traceback (most recent call last):
    *   File "./script", line 57, in <module>
    *     runScript()
    *   File "./script", line 51, in runScript
    *     print os.environ["OMERO_HOME"]
    *   File "/usr/lib64/python2.6/UserDict.py", line 22, in __getitem__
    *     raise KeyError(key)
    * KeyError: 'OMERO_HOME'
    * 
    *** end stderr ***


    *** out parameters ***
    ***  done ***

@will-moore
Copy link
Copy Markdown
Member

I'm not sure what "test.py" is in the description: bin/omero script params /test.py. I don't see a test.py in this PR, and I'm not sure if this is supposed to test what you see when 'test.py' is not found? Or if test.py should have syntax errors etc?

@joshmoore
Copy link
Copy Markdown
Member Author

  • This PR is against dev_5_0, so gretzky?
  • /test.py is just some script that fails. I don't really want to ship one by default. 😄 I used: echo foo >> lib/scripts/test.py as well as a small script with just print >>sys.stdout, "stdout"; print >>sys.stderr, "stderr"

@joshmoore joshmoore mentioned this pull request Mar 19, 2014
@will-moore
Copy link
Copy Markdown
Member

Sorry - yes, worked fine on gretzky:

OMERO_HOME
/home/omero/slave/workspace/OMERO-5.0-merge-daily/src/dist

Got stderr and stdout on invalid script:

$ omero script upload --official test.py 
Using session aaf1b3f5-fbc5-4464-a39c-8719798fc130 (root@gretzky.openmicroscopy.org.uk:4064). Idle timeout: 10.0 min. Current group: read-only-1
ApiUsageException: Can't find params for 57209!
stdout is in file 57210:
---------------------------------
stdout

---------------------------------
stderr is in file 57211:
---------------------------------
stderr

---------------------------------

@sbesson
Copy link
Copy Markdown
Member

sbesson commented Mar 20, 2014

Quickly discussed with @joshmoore. Overall, I have a very supportive feeling for providing shortcuts to omero admin ice server [start|stop] subcommands. Our typical usage is obviously the restart of the Processor-0.
In terms of naming, bin/omero admin kill does not convey the idea that the Ice process may restart and can be misleading. Unifying this feature as part of the omero admin start/stop subcommands would be the sysadmin-friendliest option. The main caveat here is that these commands already have a complex signature with optional TARGET arguments.
I would be inclined to:

  • push this feature out of this imagej PR as it does not preclude merging the other features
  • quickly brainstorm about possible signatures for this process-specific shortcut and validate them via interested parties /cc @manics @kennethgillen
  • get it implemented and merged for 5.0.1 if possible

@joshmoore joshmoore mentioned this pull request Mar 20, 2014
@joshmoore
Copy link
Copy Markdown
Member Author

Ok. I'm removed the admin kill commits from this PR (and moved them to #2187). With that, every functional change here has been tested. The addition of org.bushe to logback.xml is mostly informative for the future.

Leaving this to someone else to glance over one more time and then merge.

@bpindelski
Copy link
Copy Markdown

All changes look OK. I think this is OK to merge.

joshmoore added a commit that referenced this pull request Apr 7, 2014
Minor adjustments for imagej-omero
@joshmoore joshmoore merged commit a8fb81f into ome:dev_5_0 Apr 7, 2014
@joshmoore joshmoore deleted the imagej-omero-0.1.0 branch April 7, 2014 15:26
@joshmoore
Copy link
Copy Markdown
Member Author

--rebased-to #2263

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants