search

marpaia / chef-golang

Go library to interact with the Chef server API
Other
77 stars 31 forks source link

ConnectUrl seems unused. #16

Closed spheromak closed 10 years ago

spheromak commented 10 years ago

Maybe related to #7. Looks like this public function is unused. I think the intent is a func that returns a chef object without having parsed a knife config?

marpaia commented 10 years ago

This was added by @bakins in https://github.com/marpaia/chef-golang/pull/2

It seems like the only functionality this adds over ConnectCredentials is the ability to leave off the port when passing it a FQDN. Doesn't actually seem that useful in retrospect.

@bakins, are we missing something?

bakins commented 10 years ago

It also allows passing in path information that is needed with enterprise chef - https://chef.server/organizations/myorg

marpaia commented 10 years ago

@bakins couldn't you pass ConnectCredentials "chef.server/organizations/myorg" as host and 443 as port?

spheromak commented 10 years ago

ah yea @bakins @marpaia i think we should have one connect func that can handle these cases. We should be handling enterprise/hosted as well.

Easy enough to add a test case against hosted chef into the test suite.

spheromak commented 10 years ago

the changes i did to the backend request handling and signing should support this without much issue.

marpaia commented 10 years ago

@spheromak does goiardi support enterprise chef as well?

spheromak commented 10 years ago

don't believe so @ctdk would know.

ctdk commented 10 years ago

I think technically partial search might be an enterprise chef feature, or was at one point. I haven't implemented orgs though, or any of the other enterprise features yet. I might do reporting before too long though.

marpaia commented 10 years ago

@ctdk perhaps if you made some issues as you identify unimplemented features we could take a crack at adding some features for you one day? with the amount of usefulness that we've been getting from your code, it would be rude not to give back :)

ctdk commented 10 years ago

@marpaia I'm just tickled pink that someone's actually using it, even if it's just for testing. I can totally add issues for unimplemented features as I think of them, though; I haven't been doing that since I've been the only one working on it, and I already know what I'm doing.

@spheromak has been talking about serf/etcd/consul type support being a nice thing, and reporting looks like pretty low-hanging fruit. Once I'm done with MySQL support here (which should be pretty soon -- I had a new explosion just happen though after this refactoring, but the in-memory mode all works again on that branch), I'd like to get Postgres in as well somewhere along the line. I'm certainly open to further suggestions though.

bradbeam commented 10 years ago

Not sure how non-passive we need to be ( yet ), but I've got a proposed fix for this that should address the enterprise chef coverage

robdaemon commented 10 years ago

Well, it wasn't unused. We were actually using it. ugh.

spheromak commented 10 years ago

@robdaemon is your use case not covered by the change ? Maybe this issue should have been that it is redundant. not that it is unused.

yaauie commented 10 years ago

@spheromak the issue isn't whether or not a consumer of this package can change their code to work with the current revision; it's that the removal of a public API without warning broke someone else's build.

spheromak commented 10 years ago

@yaauie I get that, and I apologize to everyone who might get bit by this and maybe some other changes to come..

The project itself is in a state of reorg on many of these endpoints. I wish there was a canonical way in Go to manage tags/refs for imports so people didn't get breaks like this while we work to bring a better api.

We talked about moving to semver, and I like the idea, but unless you are vendoring in your project then I would expect that your builds will break a bit.

@marpaia and I agreed that this particular library is still young, and if we are gonna do those breaks it's better to get it done now than later.

If however that break removes functionality that you need. We would like that input, and know that I am sincere about being sorry for the inconvenience.

Perhaps we need to spin up a ML for announcing these breaks ahead of time so that people aren't surprised. I am up for suggestions on how to make this less painful.

marpaia commented 10 years ago

@yaauie @robdaemon would you subscribe to a mailing list if we made one? what would have been your preferred way of hearing about this?

robdaemon commented 10 years ago

I would - that would be useful. I would also prefer something like gopkg.in being used, so that versions could be tagged and breaking public API changes would be easier to consume: http://labix.org/gopkg.in

spheromak commented 10 years ago

@robdaemon thanks for the link to gopkg.in checking it out now.