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

FS path changes (post-PR669 discussion) #756

Merged
merged 6 commits into from Feb 20, 2013
Merged

Conversation

mtbc
Copy link
Member

@mtbc mtbc commented Feb 18, 2013

This is something of a mixed bag and requires ome/bioformats#374 to work. Major themes, mostly arising from #669, are:

  • Josh's fix for adjusting permissions, enabling
    • change template path to have import set ID (actually timestamp) not at top level
  • import through ImportLibrary fixes paths (so import from Windows to Unix is improved)
  • the temp file manager is used by unit tests
  • BioFormats is consulted on how many parent directories of a path to retain
  • general cleanup of messy code.

mtbc and others added 6 commits February 18, 2013 12:28
The decision was to maintain a top-level user directory
which has special permission handling. The directory
(of the form `%name%_%id%`) will be placed in the "user"
group so that it is visible from all queries.

NB: This does not yet fix the rest of the code that assumes
writing to the top directory is permissible!
@joshmoore
Copy link
Member

Running on Windows, certainly seems to work initially. Importing from gretzky to bp produces:

c:\OMERO\ManagedRepository>dir root-0-1731485727\2013-02\19\OMERO-merge-develop\src\dist\a.fake
 Volume in drive C has no label.
 Volume Serial Number is 40D9-5B26

 Directory of c:\OMERO\ManagedRepository\root-0-1731485727\2013-02\19\OMERO-merge-develop\src\dist

19/02/2013  10:58                 0 a.fake
               1 File(s)              0 bytes
               0 Dir(s)  39,390,523,392 bytes free

@joshmoore
Copy link
Member

Apologies, the above isn't with a new build, but with develop. Trying again.

@mtbc
Copy link
Member Author

mtbc commented Feb 19, 2013

Great. Main change with this PR is probably in Windows client + Unix server. Before, instead of creating foo/bar/baz to import foo\bar\baz, it would have created a file foo!bar!baz!.

(#669 is probably what made Windows work nicely, but that was hardly tested and it's very plausible there's some code path still that I missed, so I'm glad it seems to be working so far.)

@joshmoore
Copy link
Member

Running against gretzky (which is definitely the merge build) it looks like there may be issues with list and listFiles:

In [6]: repo.list("/")
SecurityViolation: exception ::omero::SecurityViolation
{
    serverStackTrace = ome.conditions.SecurityViolation: No such parent dir: CheckedPath()

The same is true for . and ./

@joshmoore
Copy link
Member

Note: passed repository.py to Mark for further testing.

@joshmoore
Copy link
Member

✔️ CLI import from gretzky (Linux) to Windows server worked.

✔️ Observations for OMERO.web on Windows to Windows server:

  • Image displays fine
  • Path displayed is nice: root_0/2013-02/19/1273612912/a.fake
  • Download works fine.

✔️ Observations for OMERO.insight (webstart) on Windows to Windows server:

  • Image opens fine
  • Download works
  • Re-importing downloaded file works.
  • Image name uses full Windows-separated name ❓ (C:\Users\ome\Downloads\a.fake)

✔️ Observations for OMERO.web (gretzky/Linux) to Windows server

  • NB: Move from one image to the next, the opened path re-closes making it hard to compare. ❓
  • Otherwise, works as the Windows client.

✔️ CLI Import from Windows to gretzky/Linux server worked fine.

  • Displayed on gretzky as root_0/2013-02/19/1276390148/windows.fake
  • Image viewable. Etc.

/cc @pwalczysko, anything else you can think of? If not, barring the one or two API issues, looks good (even on Windows!)

@pwalczysko
Copy link
Member

@joshmoore : Also for this PR but mainly for future, I would like to have an access to the Windows server as well. What is the name ? (probably we can talk via e-mail about it.)

Does the stderr output work on Windows as well (scripts) ? (probably another PR, but that is what crossed my mind)

No mention which about the browser used. In general, it is always interesting to see how IE8 is doing. If this one is fine, everything is fine (usually).

Otherwise, no new ideas.

@joshmoore
Copy link
Member

@pwalczysko: thanks. In response:

  • Looked quickly under IE9 (previously Firefox (Windows) and Chrome (Mac OSX))
  • "Download additional info" doesn't seem to return on Windows ❓
  • Downloading the produced zip, though works fine.

Feel free to access the server. It's not (yet) being restarted automatically each morning, but hopefully you can remind me to do so.

@joshmoore
Copy link
Member

"Download additional info" under Windows/Firefox works fine (against the Windows server)

@mtbc
Copy link
Member Author

mtbc commented Feb 20, 2013

#767 addresses the file-listing issue.

@joshmoore
Copy link
Member

Merging now that the BF commits are in. Thanks.

joshmoore added a commit that referenced this pull request Feb 20, 2013
FS path changes (post-PR669 discussion)
@joshmoore joshmoore merged commit 781fda5 into ome:develop Feb 20, 2013
@sbesson sbesson modified the milestones: 5.0.0, 5.0.0-alpha1 Nov 29, 2017
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.

None yet

4 participants