Skip to content
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

Allow agents API to update resources that an agent is associated with (#1522) #1573

Merged
merged 1 commit into from Nov 4, 2015

Conversation

Projects
None yet
3 participants
@ketan
Copy link
Member

commented Oct 20, 2015

v2 of the Agents API now accepts environments param, and the behavior
is identical to the behavior of the resources param.

@ketan ketan added this to the Release 15.3 milestone Oct 20, 2015

if (resources != null) {
command.addCommand(new GoConfigDao.UpdateResourcesCommand(uuid, new Resources(resources)));
}

if (environments != null) {
Set<String> existingEnvironments = cruiseConfig().getEnvironments().environmentsForAgent(uuid);

This comment has been minimized.

Copy link
@arvindsv

arvindsv Oct 20, 2015

Member

I think this should be a method, by itself, especially if we're thinking of an environments API.

}

for (String environmentToAdd : environmentsToAdd) {
command.addCommand(new GoConfigDao.ModifyEnvironmentCommand(uuid, environmentToAdd, TriStateSelection.Action.add));

This comment has been minimized.

Copy link
@arvindsv

arvindsv Oct 20, 2015

Member

This can fail and throw an exception, if the environment doesn't exist. I'm not sure that will be handled gracefully. I don't see any test for that.

This comment has been minimized.

Copy link
@ketan

ketan Nov 2, 2015

Author Member

If the environment does not exist GoConfigDao::ModifyEnvironmentCommand will throw a NoSuchEnvironmentException exception via EnvironmentsConfig#named. This is already tested well.

This exception will be caught in AgentService#updateAgentAttributes along with the relevant message. As of now the server responds with a 500, but that is something I'll change to respond with a 422 instead. Here's the response as of now -

$ curl -X PATCH \
    -H "Accept: application/vnd.go.cd.v2+json" \
    -H "Content-Type: application/json" \
    -d '{
      "environments": ["foo"]
    }' \
    'http://localhost:8153/go/api/agents/10c357bf-f7ae-42ff-a61f-fe3a8575a9a7'

{
  "message": "Updating agents failed:  { Environment [foo] does not exist. }"
}

This comment has been minimized.

Copy link
@ketan

ketan Nov 2, 2015

Author Member

I've addressed the issue about status codes and updated the PR. Specifying an environment that does not exist returns a 422 instead of a 500.

@ketan ketan force-pushed the ketan:agent-api-add-to-environment branch from f85b8eb to e5c0b4a Nov 2, 2015

@arvindsv

This comment has been minimized.

Copy link
Member

commented Nov 2, 2015

Looks good to me now. Someone can accept this, or I can when the build is green.

Allow agents API to update resources that an agent is associated with (
…#1522)

v2 of the Agents API now accepts `environments` param, and the behavior
is identical to the behavior of the `resources` param.

@ketan ketan force-pushed the ketan:agent-api-add-to-environment branch from e5c0b4a to efed993 Nov 3, 2015

@ketan

This comment has been minimized.

Copy link
Member Author

commented Nov 4, 2015

@arvindsv the PR build is green, I'm not sure if you're referring to that build or the end-to-end acceptance tests going green.

@arvindsv

This comment has been minimized.

Copy link
Member

commented Nov 4, 2015

I meant the full set, not the PR build. It's nearly there (except upload installers). So, I'll accept this.

arvindsv added a commit that referenced this pull request Nov 4, 2015

Merge pull request #1573 from ketan/agent-api-add-to-environment
Allow agents API to update resources that an agent is associated with (#1522)

@arvindsv arvindsv merged commit 1ee5908 into gocd:master Nov 4, 2015

5 checks passed

build-linux-PR/build-non-server
Details
build-linux-PR/build-server
Details
build-linux-PR/check_tlb_correctness
Details
build-linux-PR/go-sdk
Details
create-maven-release-PR/create-maven-release
Details

@ketan ketan deleted the ketan:agent-api-add-to-environment branch Nov 5, 2015

@zabil zabil removed this from the Release 15.3 milestone Nov 19, 2015

@ketan ketan added this to the Release 15.3 milestone Nov 24, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.