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

make some OMERO model object text properties of unbounded length #4882

Merged
merged 9 commits into from
Oct 11, 2016

Conversation

mtbc
Copy link
Member

@mtbc mtbc commented Oct 6, 2016

What this PR does

Adjusts OMERO model object text properties to no longer be limited to 255 characters:

  • Annotation.name
  • Annotation.ns
  • Channel.lookupTable
  • Dataset.name
  • Folder.name
  • Image.name
  • ImportJob.imageName
  • ImportJob.imageDescription
  • LogicalChannel.name
  • Namespace.name
  • Namespace.displayName
  • OriginalFile.name
  • OriginalFile.hash
  • Plate.name
  • Plate.status
  • PlateAcquisition.name
  • Project.name
  • Reagent.name
  • RenderingDef.name
  • Roi.name
  • Screen.name
  • Screen.protocolDescription
  • Screen.reagentSetDescription
  • StageLabel.name
  • Well.externalDescription

Testing this PR

Try using very long names, for instance:

$ bin/omero obj get Image:1 name
Using session a6ba6c3c-9f1e-417d-9abf-a850c2208a0b (user-2@localhost:4064). Idle timeout: 10 min. Current group: private-1
Captured 1.jp2
$ bin/omero obj update Image:1 name="`echo {1..100}`"
Using session a6ba6c3c-9f1e-417d-9abf-a850c2208a0b (user-2@localhost:4064). Idle timeout: 10 min. Current group: private-1
Image:1
$ bin/omero obj get Image:1 name
Using session a6ba6c3c-9f1e-417d-9abf-a850c2208a0b (user-2@localhost:4064). Idle timeout: 10 min. Current group: private-1
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100
$ 

Without this PR the above update step would give "could not update" and Blitz-0.log would report ERROR: value too long for type character varying(255).

Related reading

http://trac.openmicroscopy.org/ome/ticket/11894
ome/design#57

@mtbc mtbc closed this Oct 6, 2016
@mtbc mtbc reopened this Oct 6, 2016
@jburel jburel added the develop label Oct 6, 2016
@jburel
Copy link
Member

jburel commented Oct 7, 2016

in screen.ome.xml
description has changed from string to text, except optional name="externalDescription" type="string"/>

@jburel
Copy link
Member

jburel commented Oct 7, 2016

is not really used but it could be bumped

@jburel
Copy link
Member

jburel commented Oct 7, 2016

Maybe sha1/orignal file hash could be extended if we are thinking of introducing other Checksum Algorithms
cc @joshmoore

@jburel
Copy link
Member

jburel commented Oct 7, 2016

if we have to include the full path to the lut instead of only the name to avoid conflict
we probably need to change <optional name="lookupTable" type="string"/>

@jburel
Copy link
Member

jburel commented Oct 7, 2016

<optional name="status" type="string"/> of a plate could be text since it is used like a description.

@jburel
Copy link
Member

jburel commented Oct 7, 2016

  • starting from a fresh DB: works fine
  • starting from a patch 12 db:
    • upgrade works
    • with patch 12: bin/omero obj update Image:1 name="echo {1..1000}" Failed to update Image:1 - could not update``
    • with patch 13: it works

@jburel
Copy link
Member

jburel commented Oct 7, 2016

Tested against: trout.openmicroscopy.org:34064 (breaking-deploy) 5.2->5.3 patch 13
``
bin/omero obj update Image:1734 name="echo {1..1000}"
Using session 1432e635-48c6-419f-8e41-c24561c85de9 (root@trout.openmicroscopy.org:34064). Idle timeout: 10 min. Current group: system
Image:1734

``

@jburel jburel mentioned this pull request Oct 7, 2016
@jburel
Copy link
Member

jburel commented Oct 10, 2016

Following this DB change PR, we need to add integration tests to cover the various fields.
e.g. checking lookup table change from CI will be tricky if not impossible

Do you want to add a card?

@jburel
Copy link
Member

jburel commented Oct 10, 2016

Moving this PR out of breaking

@mtbc
Copy link
Member Author

mtbc commented Oct 10, 2016

@dominikl
Copy link
Member

Looks good. Tested setting a long String to a couple of fields mentioned in the PR description (but not all of them), via CLI and Insight. No problems noticed.

@mtbc
Copy link
Member Author

mtbc commented Oct 11, 2016

Thank you. OMERO CI looks good too: https://ci.openmicroscopy.org/view/Failing/

@jburel jburel merged commit 8ed4bfd into ome:develop Oct 11, 2016
@mtbc mtbc deleted the trac-11894-text-properties branch October 12, 2016 06:57
@jburel jburel added this to the 5.3.0 milestone Mar 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.

3 participants