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

Add --set to config append, --report to config {set,append,remove} #4791

Merged
merged 2 commits into from
Aug 26, 2016

Conversation

manics
Copy link
Member

@manics manics commented Aug 19, 2016

What this PR does

Adds --set option to omero config append, to indicate a value should only be added if it's not already in the list, taking into account the defaults, see #4597

Adds --report option to omero config {append,set,remove}. This will always print out Changed: ... if a change was made, and nothing if no change was made.

Testing this PR

  1. Check omero config append/set/remove behaves as before.

  2. Try something like

    omero config append --set omero.web.ui.top_links '["History", "history", {"title": "History"}]'
    

    this should have no effect (E.g. omero config get, or omero web restart and check the top-links) because this value is part of the standard defaults.

  3. Try it without --set, you should see the old behaviour i.e. the duplicate value is appended

    omero config append omero.web.ui.top_links '["History", "history", {"title": "History"}]'
    
  4. Add a different value, e.g.

    omero config append --set omero.web.ui.top_links '["BBC", "http://www.bbc.co.uk/"]'
    
  5. Try omero config {append,set,remove} --report. If a change was made you should see a message such as Changed: Appended omero.web.ui.top_links:["BBC", "http://www.bbc.co.uk/"]

The idea behind this is to assist with Ansible deployments where you want to programmatically know whether or not a change was made.

Related reading

manics added 2 commits August 19, 2016 12:15
This will only add a value if it's not already in the list
See ome#4597
@manics manics changed the title Add --set option to config append Add --set to config append, --report to config {set,append,remove} Aug 19, 2016
@joshmoore
Copy link
Member

Generally looks very useful. Primary thoughts are:

  • append --set isn't very intuitive (for me). append --once ?
  • remove could use a reciprocal flag. --all ?

cc: @ximenesuk -- with @manics away, do we want to copy this to the mainline for further investigation?

@joshmoore
Copy link
Member

Merging for the 0.0.7 tag. If we decide to rename the properties, then this will need a follow-up to metadata52 (though on the mainline we can do it straight-away)

@joshmoore joshmoore merged commit 2d29bdc into ome:metadata52 Aug 26, 2016
@joshmoore joshmoore mentioned this pull request Aug 26, 2016
@joshmoore
Copy link
Member

Note: new mapr PR which makes use of this feature: ome/infrastructure#108

@atarkowska
Copy link
Member

--rebased-to #4828

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

Successfully merging this pull request may close these issues.

4 participants