Skip to content

Verify management client#11106

Closed
DorothySun216 wants to merge 3 commits intoAzure:masterfrom
DorothySun216:verifyManagementClient
Closed

Verify management client#11106
DorothySun216 wants to merge 3 commits intoAzure:masterfrom
DorothySun216:verifyManagementClient

Conversation

@DorothySun216
Copy link
Contributor

No description provided.

@joshfree joshfree added Client This issue points to a problem in the data-plane of the library. Service Bus labels Sep 1, 2020
@joshfree joshfree added this to the Backlog milestone Sep 1, 2020
<dependency>
<groupId>org.apache.httpcomponents</groupId>
<artifactId>httpclient</artifactId>
<version>4.5.8</version>
Copy link
Member

Choose a reason for hiding this comment

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

Hey, our versioning story is managed by a series of scripts to keep dependency versions aligned. Can you add this annotation and follow the instructions to update to the proper version?
https://github.com/Azure/azure-sdk-for-java/blob/master/CONTRIBUTING.md#versions-and-versioning

<!-- {x-version-update;org.apache.httpcomponents:httpclient;external_dependency} -->

import com.microsoft.azure.servicebus.primitives.ServiceBusException;
import com.microsoft.azure.servicebus.primitives.TimeoutException;
import com.microsoft.azure.servicebus.primitives.Util;
import com.microsoft.azure.servicebus.*;
Copy link
Member

Choose a reason for hiding this comment

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

Star imports are not allowed. You may have to update IntelliJ to not change it to a star import.

private ManagementClientAsync asyncClient;

public ManagementClient(ConnectionStringBuilder connectionStringBuilder) {
public ManagementClient(ConnectionStringBuilder connectionStringBuilder) throws InterruptedException, ServiceBusException {
Copy link
Member

Choose a reason for hiding this comment

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

Since this is an existing public-facing API, this will break existing customers. You could catch the checked exceptions and throw an unchecked one to main compat.

import java.net.URI;
import java.net.URISyntaxException;
import java.net.URL;
import java.net.*;
Copy link
Member

Choose a reason for hiding this comment

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

Remove star imports.

* @param clientSettings - client settings.
*/
public ManagementClientAsync(URI namespaceEndpointURI, ClientSettings clientSettings) {
// public ManagementClientAsync(URI namespaceEndpointURI, ClientSettings clientSettings) {
Copy link
Member

Choose a reason for hiding this comment

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

Remove dead code.

@@ -0,0 +1,276 @@
package com.microsoft.azure.servicebus;
Copy link
Member

Choose a reason for hiding this comment

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

Missing MIT header.

public List<Proxy> select(URI uri) {
if (uri != null
&& uri.getHost() != null
// && uri.getHost().equalsIgnoreCase(proxyHostName)
Copy link
Member

Choose a reason for hiding this comment

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

Commented out line?


import static java.nio.charset.StandardCharsets.UTF_8;

public class DaxiTest {
Copy link
Member

Choose a reason for hiding this comment

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

What is Daxi?

import static java.nio.charset.StandardCharsets.UTF_8;

public class DaxiTest {
static final Gson GSON = new Gson();
Copy link
Member

Choose a reason for hiding this comment

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

private static final?

ExecutorService executor = Executors.newCachedThreadPool();
try {
executor.invokeAny(Arrays.asList(() -> {
System.in.read();
Copy link
Member

Choose a reason for hiding this comment

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

System.in.read()? Will this block forever? Or do you expect users to interact with this test.

@DorothySun216
Copy link
Contributor Author

This is only my draft branch that I experimented something. The one that I completed is already merged: #11108

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Client This issue points to a problem in the data-plane of the library. Service Bus Track 1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants