Skip to main content

[pkg-discuss] Re: [review] 17318601 pkg needs a synchronous option for service actuators

  • From: Erik Trauschke < >
  • To:
  • Cc: Tim Foster < >
  • Subject: [pkg-discuss] Re: [review] 17318601 pkg needs a synchronous option for service actuators
  • Date: Fri, 27 Sep 2013 09:06:38 -0700



On 09/26/13 07:23 PM, Tim Foster wrote:
Hi Erik,

I had a look at this.

On 09/27/13 12:53 PM, Erik Trauschke wrote:
17318601 pkg needs a synchronous option for service actuators
https://bug.oraclecorp.com/pls/bug/webbug_print.show?c_rptno=17318601
.
.
So far I support install, update, uninstall, change-facet and
change-variant. Let me know if you think more subcommands require an
interface to this.

I wonder would it be worth including a synchronous refresh of the
system-repository as part of set-publisher by default.

I don't think there's a need to add an option to 'set-publisher' for
this, but it seems to make sense that we should have the
system-repository service immediately reflect the publisher
configuration of the GZ.  See line 682 of src/modules/client/image.py -
having it use synchronous SMF commands there might make sense (though
worth testing to be sure the right thing happens)


Other comments:

src/client.py

line 254, 263, Could we make these two options instead:

[--sync-actuators | --sync-actuators-timeout timeout]

where:

  --sync-actuators sets an infinite timeout
  --sync-actuators-timeout sets a flexible timeout

Omission of either option uses the current asynchronous actuator to fire
and using both options together would be an invalid set of options.

I'd have no issue with this approach even though it's a bit less intuitive. However, it saves a lot of typing. Anyone else have an opinion?

src/modules/client/options.py

line 86, introduces an extra blank line
line 503, worth a comment to say 0 means asynchronous
line 509, debug code?

will fix that

src/modules/client/api.py

Does this change to the client API warrant a new (compatible) API
version - I think it does (but would defer to Shawn's advice)

If so, then I think we need to update doc/client_api_versions.txt, line
106, 107 here, and any API consumers in the gate.

Yes, I think we have to do that. It popped in my head on my way home that I'd have to do that.

line 1694, can we update the docstring to say what act_timeout is, as
promised by line 1658

Will do.

src/modules/client/plandesc.py

line 674, Can we check for a valid value here, raising an exception
otherwise? I know we have value-checks in src/modules/client/options.py
as well, but I didn't think API consumers used that code.

Can do. Probably doesn't hurt.

src/modules/smf.py, fix copyright


That's all I could spot.

Thanks
Erik


[pkg-discuss] [review] 17318601 pkg needs a synchronous option for service actuators

Erik Trauschke 09/27/2013

[pkg-discuss] Re: [review] 17318601 pkg needs a synchronous option for service actuators

Tim Foster 09/27/2013

[pkg-discuss] Re: [review] 17318601 pkg needs a synchronous option for service actuators

Erik Trauschke 09/27/2013
 
 
Close
loading
Please Confirm
Close