Hostname / Port configuration options

[nasa-ammos]

Hostname / Port configuration options

[MJJoyce]

Hey @kmarwah @aywaldron

It looks like we dropped config options for hostname and port from config.yaml in ait/dsn/sle/common.py with 28ae827ee05095216c9e1ef8928eed9d72c4da8c

Any particular motivation for dropping them? I can't track the changes to a ticket but I'm assuming it's in relation to "CMD and TLM use different hostnames/ports"

Thanks!

[aywaldron]

@MJJoyce They were actually added, and then dropped. If you go to release 1.1.0_alpha they weren't in there. Right now we don't check available hostnames for availability, so these parameters are dynamic when you're doing testing and having them in either the config file or the script itself doesn't make too much difference. I think the plan is to allow for a list of hostnames in the config file when we get around to addressing ticket #64.

[aywaldron]

36f3668 is the commit they were added in, which is part of PR https://github.com/NASA-AMMOS/AIT-DSN/pull/69 (the commit you referenced, 28ae827 is later on in PR https://github.com/NASA-AMMOS/AIT-DSN/pull/69 as well).

[MJJoyce]

Thanks @aywaldron.

The config values are used in 1.0.0_alpha when I look
https://github.com/NASA-AMMOS/AIT-DSN/blob/1.1.0_alpha/ait/dsn/sle/common.py#L100

But I realize I may have been a bit vague. I was more interested in why we suddenly stopped pulling the config value in SLE.__init__. Namely here:
https://github.com/NASA-AMMOS/AIT-DSN/blob/master/ait/dsn/sle/common.py#L102

I'm fine with not pulling that from the config, and like you mentioned, that will change after #64. Was more interested in why it got dropped since I couldn't find a justification / ticket for it. More book keeping than anything =)

[aywaldron]

@MJJoyce They are referenced there but at runtime they would have defaulted to kwargs.get('hostname', None), and the example scripts passed the hostname/port in as kwargs at runtime rather than including them in the config.

I don't think there is a reason that we stopped pulling from the config there, and I agree that we should revert the init back to what it was before, pulling from the config first and defaulting to kwargs.

[MJJoyce]

+1 regarding your second paragraph, I'll update the parent to reflect that.

[MJJoyce]

Resolved in #83