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

Py2 #1

Merged
merged 7 commits into from
Aug 8, 2019
Merged

Py2 #1

merged 7 commits into from
Aug 8, 2019

Conversation

joshmoore
Copy link
Member

This PR intends to add the necessary infrastructure on top of a decoupled versions of https://github.com/ome/openmicroscopy/releases/tag/v5.5.1 (c21d65e) such that python setup.py leads to a package that can then be pip-installed.

@joshmoore
Copy link
Member Author

This is now getting to the point where I think I can try removing lib/python from an OMERO 5.5.1 installation and seeing if pip install omero-py will be fully functional. If that works, I'd propose tagging this as 5.5.1 and pushing it out to PyPI. Then the team can start adding PRs for 5.5.x while I work against a py3 integration branch that will become 5.6 or later.

Unfortunately, users will need to rm -rf lib/python to consume 5.5.1+ but perhaps that's worth it to some until another OMERO release changes the prepend behavior of:

https://github.com/ome/openmicroscopy/blob/develop/components/tools/OmeroPy/bin/omero#L77-L84

cc: @olatarkowska @manics @sbesson @will-moore

@joshmoore
Copy link
Member Author

joshmoore commented Aug 7, 2019

Three (not necessarily mutually exclusive options) with various pros and cons are:

  1. delete lib/python, pip install, set OMERODIR, use old dist/bin/omero 👍
  2. pip install with new venv/bin/omero, set OMERODIR 👍
  3. ship new OMERO 5.5.2 with updated dist/bin/omero which appends lib/python rather than prepending it 👍

@joshmoore
Copy link
Member Author

Changes to omero-server-docker for option 1 above:

...
USER root
RUN rm -rf /opt/omero/server/OMERO.server/lib/python
COPY omero-py-5.5.1.tar.gz /tmp/
RUN pip install /tmp/omero-py-5.5.1.tar.gz
RUN mkdir /opt/omero/server/OMERO.server/lib/python # Dummy for omego
ENV OMERODIR /opt/omero/server/OMERO.server
USER omero-server

Changes to omero-server-docker for option 2 above:

...
USER root
COPY omero-py-5.5.1.tar.gz /tmp/
RUN pip install /tmp/omero-py-5.5.1.tar.gz
ENV OMERODIR /opt/omero/server/OMERO.server
USER omero-server

Additionally for option 2, I'd suggest we add:

index fe2d47b..f9f619f 100755
--- a/bin/omero
+++ b/bin/omero
@@ -75,20 +75,22 @@ else:
 # This list needs to be kept in line with omero.cli.CLI._env
 #
 top = os.path.normpath(top)
 lib = os.path.join(top, "lib")
 lpy = os.path.join(top, "lib","python")
 ipy = os.path.join(top, "lib","fallback")
 var = os.path.join(top, "var")
 vlb = os.path.join(var, "lib")
-sys.path.insert(0,vlb);
-sys.path.insert(0,lpy);
+sys.path.append(lpy);
+sys.path.append(vlb);
 sys.path.append(ipy)

but note that the referenced _env already appends rather than prepends so that the new code is picked up.

@manics
Copy link
Member

manics commented Aug 8, 2019

Since they all require an openmicroscopy release option 2 seems the cleanest

@joshmoore
Copy link
Member Author

Definitely agree that 2 is the cleanest, but I was hoping option 1 and option 2 didn't require an ome/openmicroscopy release.

@joshmoore
Copy link
Member Author

Note: one thing that can't be controlled is that if the user explicitly has set PYTHONPATH then that will be preferred. ctx._env likely has the same problem, but I only see it being used by admin.py and therefore might be ignorable (or deprecatable).

@joshmoore
Copy link
Member Author

Summary:

  • Option 2 looks to be a viable way to add omero-py from pypi to an existing OMERO installation
  • The primary downside is the requirement to set OMERODIR
  • I propose that this get reviewed and then released as a pre-release on PyPI for more thorough testing (docker, etc)
  • I'd then move on to handle omero-web in the same way.

@joshmoore
Copy link
Member Author

joshmoore commented Aug 8, 2019

General 👍 up: during the call. Only outstanding TODO is likely to strip out the integration tests and rebase, which I forgot to do. Otherwise, looking to get this pre-released to pypi (propose: 5.5.dev1) ASAP and then to move on to omero-web in the same fashion. Once that's done, those directories can be deleted from ome/openmicroscopy and all unmerged PRs closed.

@joshmoore
Copy link
Member Author

Re-opening to kick off travis

@joshmoore joshmoore closed this Aug 8, 2019
@joshmoore joshmoore reopened this Aug 8, 2019
@joshmoore
Copy link
Member Author

Merging and releasing this to PyPI now. Follow-up items include:

  • running tests
  • moving to bumpversion
  • extracting gateway & plugins

Other suggestions/ideas welcome either as a card on https://github.com/orgs/ome/projects/5 or as a new PR against this repository.

@joshmoore joshmoore merged commit a4701ad into ome:master Aug 8, 2019
@joshmoore joshmoore deleted the py2 branch August 8, 2019 12:30
@joshmoore
Copy link
Member Author

Note: origin/master was force-pushed after this was merged since I forgot to run bfg first to strip out large files. The only change was:

/opt/omero-py $bash size.sh
All sizes are in kB's. The pack column is the size of the object, compressed, inside the pack file.
size   SHA                                       location
35852  3d57f7ae1bd728b6e320741ea980cc7cfc228cad  src/omero/gateway/scripts/imgs/CHOBI_d3d.dv

This was referenced Aug 9, 2019
@sbesson
Copy link
Member

sbesson commented Aug 19, 2019

As a minimal validation, I have been using the state of this repository within the context of IDR/idr-metadata#380 (comment):

$ pip install --user omero-py
$ omero login demo@localhost
$ python pyidr/study_parser.py --set ...

Everything worked as expected with the projects and datasets being annotated as expected using the pip-installed library

@joshmoore
Copy link
Member Author

Does that imply we should go ahead and tag 5.5.1? cc: @ome/python

snoopycrimecop pushed a commit to snoopycrimecop/omero-py that referenced this pull request Apr 1, 2021
This should replace the wait.sh-stype scripts that exist in
multiple (especially docker) repositories.

Example output:

```
    $ omero sessions login -w omero root@localhost --retry=100
    Previous session expired for root on localhost:4064
    09:41:30.959913: Login retry ome#1 in 3s
    09:41:33.977181: Login retry ome#2 in 3s
    ...
    09:41:58.129038: Login retry ome#10 in 3s
    WARNING:omero.client:None - createSession retry: 1
    WARNING:omero.client:None - createSession retry: 2
    09:42:17.100098: Login retry ome#11 in 3s
    Created session for root@localhost:4064. Idle timeout: 10 min. Current group: system
````

see: https://github.com/ome/omero-server-docker/blob/master/test_login.sh
jburel pushed a commit that referenced this pull request Apr 24, 2024
Show Image, Dataset and Project in a table
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants