Re: [foreman-dev] Hammer CLI Options for ID Resolution

2016-07-27 Thread Andrew Kofink
Tomas,

Here's the issue: http://projects.theforeman.org/issues/15830

The affected commands are quite widespread. Since the 6.2 release, there
have been 2 bug reports that duplicate 15830. Any command that has content
views or lifecycle environments as a non-primary option with ID resolution
(i.e. hostgroups create/update, content-view filter create/update) are most
likely affected. We really need to get this fixed before the 6.3
development freeze, and right now, hammer-cli-katello development could be
blocked if there's a non-related bug on an affected command.

I'd be happy to chat with you tomorrow. How about 3pm your time? I will
send you a calendar invite.

Andrew Kofink

Software Engineering Intern
Red Hat Satellite 6
akof...@redhat.com

On Wed, Jul 27, 2016 at 3:10 PM, Tomas Strachota 
wrote:

> Hi Andrew,
>
> searchables are meant to be used only for storing alternative identifiers
> for a resource. That's why they're always prefixed in options. Together
> with *_id params from index action of the resource they create compound
> identifier. I understand it makes it very difficult to use in your case,
> but I wouldn't change the searchables behaviour (for now). The whole thing
> deserves deeper refactoring. I put my ideas into a RFC [1] and I would be
> glad if you could add more issues you encountered with this part of code
> and review the proposal. I hope that together we can make it more readable
> and usable.
>
> Until we do the proper refactoring I'd stay with my previous proposal.
> Feel free to ping me and we can discuss it via hangout. Can you please send
> me the issue you're working on or what commands are affected? I'll take a
> look at it prior to our chat.
>
> T.
>
> [1] https://github.com/theforeman/rfcs/pull/8
>
>
> On 07/26/2016 08:30 PM, Andrew Kofink wrote:
>
>> Tomas,
>>
>> Right now, if I add a searchable for organization-id to the
>> :content_views resource, the option is scoped as
>> --content-view-organization-id. I think it may be beneficial to modify
>> the DependentSearchablesOptionBuilder in hammer-cli-foreman to accept
>> another option, scope_to_resource. This option would default to 'true',
>> and the behavior would remain as is, but if it's set to 'false', this
>> logic
>> <
>> https://github.com/theforeman/hammer-cli-foreman/blob/master/lib/hammer_cli_foreman/option_builders.rb#L232
>> >
>> would change to make the option at the top level (i.e. --organization-id
>> instead of --content-view-organization-id). Here's what I was thinking:
>> https://gist.github.com/akofink/c1682bfbdf70e848deeed7be6eb31063
>>
>> The SEARCHABLES array would accept options like this:
>>
>> SEARCHABLES = {
>>:hostgroup => [
>>  s_name(_("Hostgroup name")),
>>  s("title", _("Hostgroup title"), editable: false,
>> scope_to_resource: false)
>>]
>> }
>>
>> I wanted to run this by you before beginning work on it to make sure it
>> seems sensible. Also, since the logic wouldn't be Katello specific, the
>> change should be able to go into hammer-cli-foreman; Extending the
>> DependentSearchablesOptionBuilder from hammer-cli-katello seems like it
>> might get messy.
>>
>> ​Thanks,
>>
>> Andrew Kofink
>>
>> Software Engineering Intern
>> Red Hat Satellite 6
>> akof...@redhat.com 
>>
>> On Tue, Jul 26, 2016 at 5:01 AM, Tomas Strachota > > wrote:
>>
>> On 07/22/2016 09:01 PM, Andrew Kofink wrote:
>>
>> Hi Team (especially Martin and Tomas),
>>
>> I've been banging my head on the table for a few days now, so I
>> thought
>> I would just ask:
>>
>> I want to resolve a content view by its name; currently,
>> content_views#index has no required parameters, so no automatic
>> options
>> are generated. I would like to add the option to search for the
>> content
>> view by organization_id, and I also want hammer to add the
>> options for
>> resolving that organization_id from the organization_name and
>> organization_label. Lastly, and probably most important, I want
>> this to
>> happen any time the option --content-view-id is added to a
>> command.
>>
>> This works downstream because organization_id is a required
>> parameter on
>> content_views#index, but upstream is currently broken due to a
>> change in
>> the API that removes that requirement. Any thoughts?
>>
>>
>> Hi Andrew,
>> I don't think there's a simple way to achieve this across the whole
>> hammer. You can either create a module with such behaviour and
>> include it into each command that uses content views (annoying I
>> know) or create a custom option builder for katello commands
>> (descendant of [1]). It will probably require some tuning of the
>> resolver too, but should give you what you want in the end.
>>
>> T.
>>
>> [1]
>>
>> https://github.com/theforeman/hammer-cl

Re: [foreman-dev] Hammer CLI Options for ID Resolution

2016-07-27 Thread Tomas Strachota

Hi Andrew,

searchables are meant to be used only for storing alternative 
identifiers for a resource. That's why they're always prefixed in 
options. Together with *_id params from index action of the resource 
they create compound identifier. I understand it makes it very difficult 
to use in your case, but I wouldn't change the searchables behaviour 
(for now). The whole thing deserves deeper refactoring. I put my ideas 
into a RFC [1] and I would be glad if you could add more issues you 
encountered with this part of code and review the proposal. I hope that 
together we can make it more readable and usable.


Until we do the proper refactoring I'd stay with my previous proposal. 
Feel free to ping me and we can discuss it via hangout. Can you please 
send me the issue you're working on or what commands are affected? I'll 
take a look at it prior to our chat.


T.

[1] https://github.com/theforeman/rfcs/pull/8


On 07/26/2016 08:30 PM, Andrew Kofink wrote:

Tomas,

Right now, if I add a searchable for organization-id to the
:content_views resource, the option is scoped as
--content-view-organization-id. I think it may be beneficial to modify
the DependentSearchablesOptionBuilder in hammer-cli-foreman to accept
another option, scope_to_resource. This option would default to 'true',
and the behavior would remain as is, but if it's set to 'false', this
logic

would change to make the option at the top level (i.e. --organization-id
instead of --content-view-organization-id). Here's what I was thinking:
https://gist.github.com/akofink/c1682bfbdf70e848deeed7be6eb31063

The SEARCHABLES array would accept options like this:

SEARCHABLES = {
   :hostgroup => [
 s_name(_("Hostgroup name")),
 s("title", _("Hostgroup title"), editable: false,
scope_to_resource: false)
   ]
}

I wanted to run this by you before beginning work on it to make sure it
seems sensible. Also, since the logic wouldn't be Katello specific, the
change should be able to go into hammer-cli-foreman; Extending the
DependentSearchablesOptionBuilder from hammer-cli-katello seems like it
might get messy.

​Thanks,

Andrew Kofink

Software Engineering Intern
Red Hat Satellite 6
akof...@redhat.com 

On Tue, Jul 26, 2016 at 5:01 AM, Tomas Strachota mailto:tstrach...@redhat.com>> wrote:

On 07/22/2016 09:01 PM, Andrew Kofink wrote:

Hi Team (especially Martin and Tomas),

I've been banging my head on the table for a few days now, so I
thought
I would just ask:

I want to resolve a content view by its name; currently,
content_views#index has no required parameters, so no automatic
options
are generated. I would like to add the option to search for the
content
view by organization_id, and I also want hammer to add the
options for
resolving that organization_id from the organization_name and
organization_label. Lastly, and probably most important, I want
this to
happen any time the option --content-view-id is added to a command.

This works downstream because organization_id is a required
parameter on
content_views#index, but upstream is currently broken due to a
change in
the API that removes that requirement. Any thoughts?


Hi Andrew,
I don't think there's a simple way to achieve this across the whole
hammer. You can either create a module with such behaviour and
include it into each command that uses content views (annoying I
know) or create a custom option builder for katello commands
(descendant of [1]). It will probably require some tuning of the
resolver too, but should give you what you want in the end.

T.

[1]

https://github.com/theforeman/hammer-cli-foreman/blob/master/lib/hammer_cli_foreman/option_builders.rb#L103


Regards,

Andrew Kofink

​akofink on IRC​
Software Engineering Intern
Red Hat Satellite 6
akof...@redhat.com 
>

--
You received this message because you are subscribed to the Google
Groups "foreman-dev" group.
To unsubscribe from this group and stop receiving emails from
it, send
an email to foreman-dev+unsubscr...@googlegroups.com

>.
For more options, visit https://groups.google.com/d/optout.


--
You received this message because you are subscribed to the Google
Groups "foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it,
send an email to foreman-dev+unsubscr...@googlegr

Re: [foreman-dev] Hammer CLI Options for ID Resolution

2016-07-26 Thread Andrew Kofink
Tomas,

Right now, if I add a searchable for organization-id to the :content_views
resource, the option is scoped as --content-view-organization-id. I think
it may be beneficial to modify the DependentSearchablesOptionBuilder in
hammer-cli-foreman to accept another option, scope_to_resource. This option
would default to 'true', and the behavior would remain as is, but if it's
set to 'false', this logic

would change to make the option at the top level (i.e. --organization-id
instead of --content-view-organization-id). Here's what I was thinking:
https://gist.github.com/akofink/c1682bfbdf70e848deeed7be6eb31063

The SEARCHABLES array would accept options like this:

SEARCHABLES = {
  :hostgroup => [
s_name(_("Hostgroup name")),
s("title", _("Hostgroup title"), editable: false, scope_to_resource:
false)
  ]
}

I wanted to run this by you before beginning work on it to make sure it
seems sensible. Also, since the logic wouldn't be Katello specific, the
change should be able to go into hammer-cli-foreman; Extending the
DependentSearchablesOptionBuilder from hammer-cli-katello seems like it
might get messy.

​Thanks,

Andrew Kofink

Software Engineering Intern
Red Hat Satellite 6
akof...@redhat.com

On Tue, Jul 26, 2016 at 5:01 AM, Tomas Strachota 
wrote:

> On 07/22/2016 09:01 PM, Andrew Kofink wrote:
>
>> Hi Team (especially Martin and Tomas),
>>
>> I've been banging my head on the table for a few days now, so I thought
>> I would just ask:
>>
>> I want to resolve a content view by its name; currently,
>> content_views#index has no required parameters, so no automatic options
>> are generated. I would like to add the option to search for the content
>> view by organization_id, and I also want hammer to add the options for
>> resolving that organization_id from the organization_name and
>> organization_label. Lastly, and probably most important, I want this to
>> happen any time the option --content-view-id is added to a command.
>>
>> This works downstream because organization_id is a required parameter on
>> content_views#index, but upstream is currently broken due to a change in
>> the API that removes that requirement. Any thoughts?
>>
>
> Hi Andrew,
> I don't think there's a simple way to achieve this across the whole
> hammer. You can either create a module with such behaviour and include it
> into each command that uses content views (annoying I know) or create a
> custom option builder for katello commands (descendant of [1]). It will
> probably require some tuning of the resolver too, but should give you what
> you want in the end.
>
> T.
>
> [1]
> https://github.com/theforeman/hammer-cli-foreman/blob/master/lib/hammer_cli_foreman/option_builders.rb#L103
>
>
>> Regards,
>>
>> Andrew Kofink
>>
>> ​akofink on IRC​
>> Software Engineering Intern
>> Red Hat Satellite 6
>> akof...@redhat.com 
>>
>> --
>> You received this message because you are subscribed to the Google
>> Groups "foreman-dev" group.
>> To unsubscribe from this group and stop receiving emails from it, send
>> an email to foreman-dev+unsubscr...@googlegroups.com
>> .
>> For more options, visit https://groups.google.com/d/optout.
>>
>
> --
> You received this message because you are subscribed to the Google Groups
> "foreman-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to foreman-dev+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>

-- 
You received this message because you are subscribed to the Google Groups 
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to foreman-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: [foreman-dev] Hammer CLI Options for ID Resolution

2016-07-26 Thread Tomas Strachota

On 07/22/2016 09:01 PM, Andrew Kofink wrote:

Hi Team (especially Martin and Tomas),

I've been banging my head on the table for a few days now, so I thought
I would just ask:

I want to resolve a content view by its name; currently,
content_views#index has no required parameters, so no automatic options
are generated. I would like to add the option to search for the content
view by organization_id, and I also want hammer to add the options for
resolving that organization_id from the organization_name and
organization_label. Lastly, and probably most important, I want this to
happen any time the option --content-view-id is added to a command.

This works downstream because organization_id is a required parameter on
content_views#index, but upstream is currently broken due to a change in
the API that removes that requirement. Any thoughts?


Hi Andrew,
I don't think there's a simple way to achieve this across the whole 
hammer. You can either create a module with such behaviour and include 
it into each command that uses content views (annoying I know) or create 
a custom option builder for katello commands (descendant of [1]). It 
will probably require some tuning of the resolver too, but should give 
you what you want in the end.


T.

[1] 
https://github.com/theforeman/hammer-cli-foreman/blob/master/lib/hammer_cli_foreman/option_builders.rb#L103




Regards,

Andrew Kofink

​akofink on IRC​
Software Engineering Intern
Red Hat Satellite 6
akof...@redhat.com 

--
You received this message because you are subscribed to the Google
Groups "foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send
an email to foreman-dev+unsubscr...@googlegroups.com
.
For more options, visit https://groups.google.com/d/optout.


--
You received this message because you are subscribed to the Google Groups 
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to foreman-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[foreman-dev] Hammer CLI Options for ID Resolution

2016-07-22 Thread Andrew Kofink
Hi Team (especially Martin and Tomas),

I've been banging my head on the table for a few days now, so I thought I
would just ask:

I want to resolve a content view by its name; currently,
content_views#index has no required parameters, so no automatic options are
generated. I would like to add the option to search for the content view by
organization_id, and I also want hammer to add the options for resolving
that organization_id from the organization_name and organization_label.
Lastly, and probably most important, I want this to happen any time the
option --content-view-id is added to a command.

This works downstream because organization_id is a required parameter on
content_views#index, but upstream is currently broken due to a change in
the API that removes that requirement. Any thoughts?

Regards,

Andrew Kofink

​akofink on IRC​
Software Engineering Intern
Red Hat Satellite 6
akof...@redhat.com

-- 
You received this message because you are subscribed to the Google Groups 
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to foreman-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.