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

bin/omero entrypoint #229

Merged
merged 5 commits into from
Jul 9, 2020
Merged

Conversation

manics
Copy link
Member

@manics manics commented Jun 30, 2020

Closes #219

Converts bin/omero into an entrypoint (cross-platform, so *.bat files are no longer needed).

Testing: omero CLI should work as before.

There are probably loads of other cleanups that could be done such as removing the path detection (sys.path.append(vlb)) now that OMERODIR is required for anything server side, but I've tried to keep the changes minimal for now.

bin/omero was renamed to src/omero/main.py, it looks like a new file because I had to wrap most of the script in a main() function but it's mostly indentation and flake8 fixes.

Could do with someone testing this on Windows

@joshmoore
Copy link
Member

================ 1171 passed, 154 skipped, 1 xfailed in 33.78s ================
4564___________________________________ summary ___________________________________
4565  py37-win: commands succeeded

So tests are certainly running on travis. It'd be good to review that bin/omero tests are skipped on Windows, though.

@joshmoore
Copy link
Member

Changes (with ignore whitespace) all look good.

@dominikl
Copy link
Member

dominikl commented Jul 1, 2020

Testing on Windows10 VM.

C:\Users\IEUser\omero-py>omero --help
Traceback (most recent call last):
  File "C:\Users\IEUser\AppData\Local\Programs\Python\Python38\Scripts\omero-script.py", line 11, in <module>
    load_entry_point('omero-py==5.7.2.dev0', 'console_scripts', 'omero')()
  File "c:\users\ieuser\appdata\local\programs\python\python38\lib\site-packages\omero\main.py", line 69, in main
    exe = readlink()
  File "c:\users\ieuser\appdata\local\programs\python\python38\lib\site-packages\omero\main.py", line 52, in readlink
    while stat.S_ISLNK(os.lstat(file)[stat.ST_MODE]):
FileNotFoundError: [WinError 2] The system cannot find the file specified: 'C:\\Users\\IEUser\\AppData\\Local\\Programs\\Python\\Python38\\Scripts\\omero'

@manics
Copy link
Member Author

manics commented Jul 1, 2020

Looks like bin/omero was found and executed but it's now failing in readlink() during the path detection:

def readlink(file=sys.argv[0]):
    """
    Resolve symlinks and similar. This is useful to allow
    linking this file under /usr/bin/, for example.
    """
    import stat

    file = sys.argv[0]
    while stat.S_ISLNK(os.lstat(file)[stat.ST_MODE]):   #### Failing on this line
        target = os.readlink(file)
        if target[0] != "/":
            file = os.path.join(os.path.dirname(file), target)
        else:
            file = target

    file = os.path.abspath(file)
    return file
    exe = readlink()
    top = os.path.join(exe, os.pardir, os.pardir)

    #
    # This list needs to be kept in line with omero.cli.CLI._env
    #
    top = os.path.normpath(top)
    var = os.path.join(top, "var")
    vlb = os.path.join(var, "lib")
    sys.path.append(vlb)

@joshmoore Is it safe to remove, on the assumption that OMERODIR will be set for anything requiring that path?

@joshmoore
Copy link
Member

@joshmoore Is it safe to remove,

It's similar to OMERO_HOME in that it will be the removal of a little known feature, but a removal nonetheless.

@manics
Copy link
Member Author

manics commented Jul 1, 2020

If I'm reading the code correctly, assuming exe = /path/to/venv/bin/omero:

  • top = /path/to/venv
  • vlb = /path/to/venv/var/lib

What's the expected use of /path/to/venv/var/lib ?

@joshmoore
Copy link
Member

Dim memory of a place to put libs for scripts?

@manics
Copy link
Member Author

manics commented Jul 2, 2020

This is now working on Windows, I've tested it in a VM with omero login wss://idr.openmicroscopy.org/omero-ws

There are still loads of other warnings/errors since Windows isn't (yet) properly supported by omero-py.

@dominikl
Copy link
Member

dominikl commented Jul 2, 2020

👍

C:\Users\IEUser\omero-py>omero login
Server: [localhost:4064]demo.openmicroscopy.org
Username: [IEUser]dlindner
Password:
Created session for dlindner@demo.openmicroscopy.org:4064. Idle timeout: 10 min. Current group: 2018-02

@snoopycrimecop
Copy link
Member

Conflicting PR. Removed from build OMERO-python-superbuild-push#365. See the console output for more details.
Possible conflicts:

--conflicts

@snoopycrimecop
Copy link
Member

snoopycrimecop commented Jul 4, 2020

Conflicting PR. Removed from build OMERO-python-superbuild-push#366. See the console output for more details.
Possible conflicts:

--conflicts Conflict resolved in build OMERO-python-superbuild-push#369. See the console output for more details.

@manics manics mentioned this pull request Jul 6, 2020
4 tasks
Copy link
Member

@joshmoore joshmoore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code changes all look good to me.

@joshmoore joshmoore merged commit c347426 into ome:master Jul 9, 2020
@manics manics deleted the omero-entrypoint branch July 9, 2020 13:19
@manics manics mentioned this pull request Jul 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet