SLCORE-1110 sq ide connected mode works with multi regions support in sqc #1225
Conversation
kirill-knize-sonarsource
force-pushed
the
feature/kk/SLCORE-1110-SQ-IDE-Connected-Mode-works-with-multi-regions-support-in-SQC
branch
from
00bf001 to
1295e66
Compare
sophio-japharidze-sonarsource
force-pushed
the
feature/kk/SLCORE-1110-SQ-IDE-Connected-Mode-works-with-multi-regions-support-in-SQC
branch
4 times, most recently
from
69280bb to
668f1bd
Compare
…oudActiveEnvironment
kirill-knize-sonarsource
force-pushed
the
feature/kk/SLCORE-1110-SQ-IDE-Connected-Mode-works-with-multi-regions-support-in-SQC
branch
from
f8232a3 to
d234a0d
Compare
|
| @@ -219,7 +220,7 @@ private static BindingProperties extractConnectionProperties(ClientFile matchedF | |||
| return null; | |||
| } | |||
| return new BindingProperties(getAndTrim(properties, "sonar.projectKey"), getAndTrim(properties, "sonar.organization"), | |||
| getAndTrim(properties, "sonar.host.url"), false); | |||
| getAndTrim(properties, "sonar.host.url"), null, false); | |||
There was a problem hiding this comment.
Do you know what is the plan to support the US region regarding the .sonarcloud.properties file? Should users specify a different sonar.host.url? There might be something to do here other than setting the region to null
| try { | ||
| configuredRegion = region != null ? SonarCloudRegion.valueOf(region) : SonarCloudRegion.EU; | ||
| } catch (IllegalArgumentException e) { | ||
| LOG.warn("Unknown region '{}'", region); |
There was a problem hiding this comment.
Could you give a bit more details in the trace? It's a bit short and lacks context
| } | ||
| } | ||
|
|
||
| @CheckForNull | ||
| private BindingClue computeBindingClue(String filename, BindingProperties scannerProps) { | ||
| if (AUTOSCAN_CONFIG_FILENAME.equals(filename)) { | ||
| return new SonarCloudBindingClue(scannerProps.projectKey, scannerProps.organization, scannerProps.isFromSharedConfiguration); | ||
| return new SonarCloudBindingClue(scannerProps.projectKey, scannerProps.organization, null, scannerProps.isFromSharedConfiguration); |
There was a problem hiding this comment.
Same question here, is there a way to figure out the region?
| if (removeEnd(scannerProps.serverUrl, "/").equals(removeEnd(sonarCloudUri.toString(), "/"))) { | ||
| return new SonarCloudBindingClue(scannerProps.projectKey, null, scannerProps.isFromSharedConfiguration); | ||
| if (sonarCloudActiveEnvironment.isSonarQubeCloud(scannerProps.serverUrl)) { | ||
| return new SonarCloudBindingClue(scannerProps.projectKey, null, scannerProps.region, scannerProps.isFromSharedConfiguration); |
There was a problem hiding this comment.
If I'm not wrong, the scannerProps.region will be null here in some cases, while the URL might be detected as the US region, and the null will lead to selecting the EU region, is it the case?
| @@ -346,6 +359,10 @@ public String getOrganization() { | |||
| return organization; | |||
| } | |||
|
|
|||
| public SonarCloudRegion getRegion() { | |||
There was a problem hiding this comment.
Should be annotated @CheckForNull?
| public GetOrganizationParams(Either<TokenDto, UsernamePasswordDto> credentials, String organizationKey) { | ||
| this(credentials, organizationKey, SonarCloudRegion.EU); |
There was a problem hiding this comment.
Same, deprecation to be documented.
| public ListUserOrganizationsParams(Either<TokenDto, UsernamePasswordDto> credentials) { | ||
| this(credentials, SonarCloudRegion.EU); |
There was a problem hiding this comment.
Same, deprecation to be documented
| public SonarCloudConnectionParams(String organizationKey, @Nullable String tokenName, @Nullable String tokenValue) { | ||
| this(organizationKey, tokenName, tokenValue, SonarCloudRegion.EU); |
There was a problem hiding this comment.
Here I'm not sure we need a deprecation. Clients are not supposed to call the constructor, they will receive an instance. But maybe for safety we can keep the deprecation. If we keep it, we should document it
| public SonarCloudConnectionSuggestionDto(String organization, String projectKey) { | ||
| this(organization, projectKey, SonarCloudRegion.EU); |
|
|
||
| public SonarCloudActiveEnvironment(URI uri, URI webSocketsEndpointUri) { | ||
| this.uri = uri; | ||
| this.webSocketsEndpointUri = webSocketsEndpointUri; | ||
| this.uris = Map.of(SonarCloudRegion.EU, new SonarQubeCloudUris(uri, webSocketsEndpointUri)); |
There was a problem hiding this comment.
This constructor is used if clients provide a custom URI. The risk here is that we don't define the US region in the map. Should we allow clients to also customize US URIs?
For SonarSourcers:
SLCORE-XXXXif you already have a ticket in Jirasnake_case, for example:test_validate_input.For external contributors:
In addition to the above, please review our contribution guidelines and ensure your pull request adheres to the following guidelines: