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

Introduce a simple VSTS REST client #88

Merged
merged 8 commits into from
Jul 19, 2016
Merged

Introduce a simple VSTS REST client #88

merged 8 commits into from
Jul 19, 2016

Conversation

olivierdagenais
Copy link
Member

@olivierdagenais olivierdagenais commented Jul 19, 2016

In preparation for a fair amount of interactions with VSTS, created the VstsRestClient class as a façade. A sample of its use can be found in VstsCollectionConfiguration, for testing the collection URL paired with the credentials.

Manual testing

  1. Temporarily re-enabled the ping() test and ran it with the necessary parameters to connect to a VSTS account. The ping succeeded.
  2. Launched a test Jenkins instance and navigated to the global configuration. For a VSTS collection URL, clicked the [Test connection] button for:
    1. Valid credentials: Success!
    2. Invalid credentials: Error: Server returned HTTP response code: 401 for URL: https://{collectionUrl}/_apis/connectiondata
    3. No credentials: Error: VSTS accounts need credentials, preferably a Personal Access Token

Mission accomplished!

TRACE,
;

private static final Charset UTF8 = Charset.forName("UTF-8");
Copy link
Contributor

@davidstaheli davidstaheli Jul 19, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using java.nio.charset.StandardCharsets.UTF_8 instead.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good idea. StandardCharsets is Java 1.7+ and we're currently targeting Java 1.6, so I'll add an item in the backlog to upgrade to Java 1.7 (as most of Jenkins has) and to make that change at the same time.

@davidstaheli
Copy link
Contributor

Looks good! See 2 minor comments.

@olivierdagenais
Copy link
Member Author

Looks good! See 2 minor comments.

Thank you for the review!

@olivierdagenais olivierdagenais merged commit ced6fb9 into master Jul 19, 2016
@olivierdagenais olivierdagenais deleted the rest_client branch July 19, 2016 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants