Skip to content
This repository has been archived by the owner on Jan 25, 2022. It is now read-only.

Add the app name to the bulk API response. #5

Merged

Conversation

andreacampi
Copy link
Contributor

Specs are green.

The app name is not so confidential or so implementation-specific that we need to hide it.
Vice versa it is very useful to include in the bulk API as it makes it possible to correlate a droplet to an app.

@d
Copy link
Contributor

d commented Feb 17, 2013

Hi Andrea,
What's your use case for this change? The targeted audience of bulk api is health manager and HM doesn't care about an app's name (which isn't globally unique).

Jesse

@andreacampi
Copy link
Contributor Author

I have a "CF Ops console" (https://github.com/andreacampi/InsideCF) that among other things lists running apps per DEA, regardless of the user that started them. This is mostly to aid in tracking down where an app is running.
As most of our infrastructure, this is not meant for end users, only for our Ops staff.

The way it works is to listen to CC and DEA announcements on NATS; but these only include app id and droplet id.
Humans are not particularly good at thinking and talking about UUID's, so the app name is a useful piece of information.
Unfortunately the app name is never visible (other than by accessing the CC DB, which I would like to avoid).

The app name used to be included in bulk api until commit cbcd543 that changed this code to an explicit whitelist.

I do get your point about the app name not being globally unique.
In my use case, this is mostly for convenience so it doesn't really have to be--the operator looking at the console knows to verify that. Still, it's usually "good enough".
As soon as I move to CC v2 I will look into also showing the organization and space (that should make it globally unique I think).

@cf-frontend
Copy link
Contributor

have you tested this with HM next?
btw even if we merge this, CCNG will not send this attribute out, can you make sure your console will not blow up when name is absent?

@andreacampi
Copy link
Contributor Author

have you tested this with HM next?

Good question, I think I have but I will verify again.

btw even if we merge this, CCNG will not send this attribute out, can you make sure your console will not blow up when name is absent?

Indeed InsideCF does not blow up--it just doesn't show this piece of information.
As soon as I switch to CCNG I will also make this change there, and submit a pull request for that too :)

@andreacampi
Copy link
Contributor Author

I can confirm this works with HM next.
In my private installation of CF I have the CC with this patch applied, and I've been running HM next for a few days. I also have my InsideCF running that makes use of this info.
It all works nicely together, no errors from the HM when fetching bulk info.

cf-frontend pushed a commit that referenced this pull request Feb 21, 2013
Add the app name to the bulk API response.

Merged by Matthew Boedicker and April Yu.
@cf-frontend cf-frontend merged commit 3d92ce2 into cloudfoundry-attic:master Feb 21, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants