Bug #5747

[RFE] Fine grained control over name expansion

Added by Bryan Kearney about 4 years ago. Updated about 4 years ago.

Status:Closed
Priority:High
Assignee:Tomáš Strachota
Category:Hammer core
Target version:-
Difficulty: Team Backlog:
Triaged: Fixed in Releases:
Bugzilla link:1101657 Found in Releases:
Pull request:

Description

I would like to have fine grained control over what *-id parameters are expanded in hammer commands. Take the command

subscription upload

for example. With some tweaks today, the parameters exposed based off the index options are:

--activation-key ACTIVATION_KEY_NAME
--activation-key-id ACTIVATION_KEY_ID
--async Do not wait for the task
--file MANIFEST Subscription manifest file
--name NAME Name to search by
--organization ORGANIZATION_NAME
--organization-id ORGANIZATION_ID Organization id
--organization-label ORGANIZATION_LABEL
--repository-url REPOSITORY_URL repository url
--system SYSTEM_NAME
--system-id SYSTEM_ID

if you compare this to the actual resource,

activation-key
name
system

are not in the update command. All I would like to have expanded are organization

Associated revisions

Revision fcbdee0d
Added by Tomas Strachota about 4 years ago

Refs #5747 - build_options configurable with block

Revision 6f198826
Added by Tomas Strachota about 4 years ago

Merge pull request #115 from tstrachota/opts

Refs #5747 - build_options configurable with block

Revision 93a278a9
Added by Tomas Strachota about 4 years ago

Fixes #5747 - Fine grained control over name expansion

Revision 7264e7b2
Added by Tomas Strachota about 4 years ago

Refs #5747 - Documentation for fine grained control over name expansion

Revision ac7deb14
Added by Tomas Strachota about 4 years ago

Merge pull request #125 from tstrachota/opts2

Fixes #5747 - Fine grained control over name expansion

History

#1 Updated by Bryan Kearney about 4 years ago

FYI.. my expectation is that the following command will execute

bundle exec bin/hammer -u admin -p changeme subscription upload --organization-label DemoOrg --file /tmp/manifest.zip --repository-url 'https://cdn.redhat.com'

#2 Updated by Adam Price about 4 years ago

tomas, do you think this should all be rolled up into the build_options command?

maybe something like

build_options(:expansion => {:only => [:organizations]})

# or

build_options.expand.only(:organizations)

and

build_options(:expansion => {:except => [:activation_keys]})

# or

build_options.expand.all.except(:activation_keys)

i prefer the second way because it reads more like english, but i have a feeling that way would be more difficult to implement.

this would still work with the current options that we may want to pass to build_options like :without, etc.

build_options(:without => [:system_id]).expand.all.except(:activation_keys)

#3 Updated by Tomáš Strachota about 4 years ago

My first idea was to simply extend the command with

searchables_for :activation_keys

which would add custom option builder.

Your idea look more elegant. I also like the fact it is separated from the command. I'd go for:

build_options.without(:system_id).expand(:all)
build_options(:without => [:system_id], :expand => { :except => :activation_keys })
# or
build_options.without(:system_id).expand(:except => :activation_keys)
build_options(:without => [:system_id], :expand => { :except => :activation_keys })

Not sure if it's not over-engineered though. It also brings difficulties with when to trigger the build process. You're calling methods on an object returned form the function that actually builds it currently.

The way out of it could be something like:

build_options do
  without(:system_id)
  expand(:activation_keys, :operatingsystems)
end

#4 Updated by Adam Price about 4 years ago

that could work.

build_options do |o|
  o.without(:system_id)
  o.expand(:activation_keys, :operatingsystems)
end

I like that. and it also solves our problem of calling methods on an already returned method. This way, it all happens inside of build_options.

#5 Updated by Tomáš Strachota about 4 years ago

Cool. The argument "o" can be a very simple object that builds the config hash. I like that -> agreed:)

#6 Updated by Adam Price about 4 years ago

i like it! +1

#7 Updated by Tomáš Strachota about 4 years ago

  • Status changed from New to Assigned

#8 Updated by Bryan Kearney about 4 years ago

  • Bugzilla link set to https://bugzilla.redhat.com/show_bug.cgi?id=1101657

#9 Updated by Anonymous about 4 years ago

  • Status changed from Assigned to Closed
  • % Done changed from 0 to 100

Applied in changeset hammer-cli-foreman|commit:93a278a998e559b74808484ecdd3140859dba76e.

Also available in: Atom PDF