-
Notifications
You must be signed in to change notification settings - Fork 128
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support for fetching files from task sandbox #356
Comments
Hey @geoffrhodes, I don't think it's problematic to add support for DC/OS-only features as long as we clearly describe them as such (e.g., leave a GoDoc comment). Is the Task struct you intend to attach the new functionality on rooted in both Marathon and DC/OS? If so, then I'm okay with using that. OTOH, if the Task struct is more of an arbitrary place because no other seems more appropriate to hook up DC/OS methods, then I'd be inclined to create a new struct instead. I'm not super familiar with DC/OS but it sounds like the former is the case? |
Hey Tim, I appreciate the feedback. After digging at it a little bit it isn't as clean to add to this package as I thought. The way the base path is added to the cluster members makes it difficult to hit non-Marathon endpoints without adding more overarching Mesos / DCOS support or refactoring the existing client endpoints to allow overriding the base path. |
hey @geoffrhodes, thanks for the update. I haven't worked with DCOS myself but I know we added at least some level of support for it to go-marathon in the past. If you check out the README here you should find an example on how to configure a client with a custom URL and DCOS token, seemingly needed for DCOS. Not sure if that helps or if there's more that'd be needed, mentioning it just in case. |
Hello,
I want to add support for list and fetching files from a task's sandbox via the endpoints described here: https://docs.mesosphere.com/1.12/monitoring/logging/logging-api/#/
I was not sure how you felt about supporting DC/OS specific endpoints which this seems to be. The most appropriate place I see to add this would be as member functions on the Task struct. If this all sounds appropriate I will write it & PR.
Thanks,
Geoff
The text was updated successfully, but these errors were encountered: