[CDAP-20993] Namespace and Repository Source control metadata refresh #15641
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| /* | ||
| * Copyright © 2024 Cask Data, Inc. | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); you may not | ||
| * use this file except in compliance with the License. You may obtain a copy of | ||
| * the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT | ||
| * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the | ||
| * License for the specific language governing permissions and limitations under | ||
| * the License. | ||
| */ | ||
|
|
||
| package io.cdap.cdap.app.store; | ||
|
|
||
| import io.cdap.cdap.proto.SourceControlMetadataRecord; | ||
| import java.util.List; | ||
|
|
||
| /** | ||
| * Represents a response containing a list of source control metadata records, next page token and | ||
| * last refresh time. | ||
| */ | ||
|
|
||
| public class ListSourceControlMetadataResponse { | ||
|
|
||
| private final List<SourceControlMetadataRecord> apps; | ||
| private final String nextPageToken; | ||
| private final Long lastRefreshTime; | ||
|
|
||
| /** | ||
| * Constructs a ListSourceControlMetadataResponse object. | ||
| * | ||
| * @param apps The list of source control metadata records. | ||
| * @param nextPageToken The token for fetching the next page of results. | ||
| * @param lastRefreshTime The timestamp of the last refresh operation. | ||
| */ | ||
| public ListSourceControlMetadataResponse(List<SourceControlMetadataRecord> apps, | ||
| String nextPageToken, Long lastRefreshTime) { | ||
|
There was a problem hiding this comment. Annotate parameters that can be null with |
||
| this.apps = apps; | ||
| this.nextPageToken = nextPageToken; | ||
| this.lastRefreshTime = lastRefreshTime; | ||
| } | ||
|
|
||
| public List<SourceControlMetadataRecord> getApps() { | ||
| return apps; | ||
| } | ||
|
|
||
| public String getNextPageToken() { | ||
|
There was a problem hiding this comment. Are these getter methods used? If not, no need to define them. |
||
| return nextPageToken; | ||
| } | ||
|
|
||
| public Long getLastRefreshTime() { | ||
|
|
||
| return lastRefreshTime; | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| /* | ||
| * Copyright © 2024 Cask Data, Inc. | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); you may not | ||
| * use this file except in compliance with the License. You may obtain a copy of | ||
| * the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT | ||
| * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the | ||
| * License for the specific language governing permissions and limitations under | ||
| * the License. | ||
| */ | ||
|
|
||
| package io.cdap.cdap.app.store; | ||
|
|
||
| import io.cdap.cdap.proto.SourceControlMetadataRecord; | ||
|
|
||
| /** | ||
| * Represents a response containing a single source control metadata record and last refresh time. | ||
| */ | ||
| public class SingleSourceControlMetadataResponse { | ||
|
|
||
| private final SourceControlMetadataRecord app; | ||
| private final Long lastRefreshTime; | ||
|
There was a problem hiding this comment. Is this nullable? If so, please annotate accordingly. |
||
|
|
||
| /** | ||
| * Constructs a SingleSourceControlMetadataResponse object. | ||
| * | ||
| * @param app The source control metadata record. | ||
| * @param lastRefreshTime The timestamp of the last refresh operation. | ||
| */ | ||
| public SingleSourceControlMetadataResponse(SourceControlMetadataRecord app, | ||
| Long lastRefreshTime) { | ||
| this.app = app; | ||
| this.lastRefreshTime = lastRefreshTime; | ||
| } | ||
|
|
||
| public SourceControlMetadataRecord getApp() { | ||
| return app; | ||
| } | ||
|
|
||
| public Long getLastRefreshTime() { | ||
| return lastRefreshTime; | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,8 +33,10 @@ | |
| import io.cdap.cdap.api.security.AccessException; | ||
| import io.cdap.cdap.app.runtime.ProgramRuntimeService; | ||
| import io.cdap.cdap.app.store.ApplicationFilter; | ||
| import io.cdap.cdap.app.store.ListSourceControlMetadataResponse; | ||
| import io.cdap.cdap.app.store.ScanApplicationsRequest; | ||
| import io.cdap.cdap.app.store.ScanSourceControlMetadataRequest; | ||
| import io.cdap.cdap.app.store.SingleSourceControlMetadataResponse; | ||
| import io.cdap.cdap.common.ApplicationNotFoundException; | ||
| import io.cdap.cdap.common.ArtifactAlreadyExistsException; | ||
| import io.cdap.cdap.common.BadRequestException; | ||
|
|
@@ -43,7 +45,9 @@ | |
| import io.cdap.cdap.common.NamespaceNotFoundException; | ||
| import io.cdap.cdap.common.NotFoundException; | ||
| import io.cdap.cdap.common.NotImplementedException; | ||
| import io.cdap.cdap.common.RepositoryNotFoundException; | ||
| import io.cdap.cdap.common.ServiceException; | ||
| import io.cdap.cdap.common.SourceControlMetadataNotFoundException; | ||
| import io.cdap.cdap.common.app.RunIds; | ||
| import io.cdap.cdap.common.conf.CConfiguration; | ||
| import io.cdap.cdap.common.conf.Constants; | ||
|
|
@@ -129,7 +133,6 @@ | |
| * Key in json paginated applications list response. | ||
| */ | ||
| public static final String APP_LIST_PAGINATED_KEY = "applications"; | ||
|
|
||
| /** | ||
| * Runtime program service for running and managing programs. | ||
|
|
@@ -295,7 +298,8 @@ | |
| } | ||
|
|
||
| /** | ||
| * Retrieves all namespace source control metadata for applications within the specified namespace, | ||
| * next page token and last refresh time. | ||
| * | ||
| * @param request The HTTP request containing parameters for retrieving metadata. | ||
| * @param responder The HTTP responder for sending the response. | ||
|
|
@@ -319,31 +323,26 @@ | |
| @QueryParam("filter") String filter | ||
| ) throws Exception { | ||
| validateNamespace(namespaceId); | ||
| List<SourceControlMetadataRecord> apps = new ArrayList<>(); | ||
|
There was a problem hiding this comment. we should check if repoconfig is present here |
||
| ScanSourceControlMetadataRequest scanRequest = SourceControlMetadataHelper.getScmStatusScanRequest( | ||
| namespaceId, | ||
| pageToken, pageSize, sortOrder, sortOn, filter); | ||
| boolean pageLimitReached; | ||
| pageLimitReached = applicationLifecycleService.scanSourceControlMetadata( | ||
| scanRequest, batchSize, | ||
| apps::add); | ||
|
There was a problem hiding this comment. In the old code we don't buffer the app, but in the new code we do. I remember do removed the buffered to avoid high memory consumption, but now we are bringing the same problem back. Is buffering needed? Seems like we can refactor the JsonPaginatedListResponder so that it can emit extra fields (lastRefreshTime in this case) before closing the json object. There was a problem hiding this comment. We can extend |
||
| SourceControlMetadataRecord record = apps.isEmpty() ? null :apps.get(apps.size() - 1); | ||
|
Check warning on line 334 in cdap-app-fabric/src/main/java/io/cdap/cdap/gateway/handlers/AppLifecycleHttpHandler.java
|
||
| String nextPageToken = !pageLimitReached || record == null ? null : | ||
| record.getName(); | ||
| long lastRefreshTime = applicationLifecycleService.getLastRefreshTime(namespaceId); | ||
|
There was a problem hiding this comment. Seems like unwise to sync all apps, shouldn't we just sync the apps fetched in this page? There was a problem hiding this comment. As we are cloning the full repository which would be time taking it might be wasteful to just update few pipelines. |
||
| ListSourceControlMetadataResponse response = new ListSourceControlMetadataResponse(apps, | ||
| nextPageToken, lastRefreshTime == 0L ? null : lastRefreshTime); | ||
| responder.sendJson(HttpResponseStatus.OK, GSON.toJson(response)); | ||
| } | ||
|
|
||
| /** | ||
| * Retrieves the source control metadata for the specified application within the specified namespace | ||
| * and last refresh time of git pull/push. | ||
| * | ||
| * @param request The HTTP request containing parameters for retrieving metadata. | ||
| * @param responder The HTTP responder for sending the response. | ||
|
|
@@ -355,12 +354,15 @@ | |
| @Path("/apps/{app-id}/sourcecontrol") | ||
| public void getNamespaceSourceControlMetadata(HttpRequest request, HttpResponder responder, | ||
| @PathParam("namespace-id") final String namespaceId, | ||
| @PathParam("app-id") final String appName) | ||
| throws BadRequestException, NamespaceNotFoundException, | ||
| SourceControlMetadataNotFoundException, RepositoryNotFoundException { | ||
| validateApplicationId(namespaceId, appName); | ||
| SourceControlMetadataRecord app = applicationLifecycleService.getSourceControlMetadataRecord( | ||
| new ApplicationReference(namespaceId, appName)); | ||
| Long lastRefreshTime = applicationLifecycleService.getLastRefreshTime(namespaceId); | ||
| SingleSourceControlMetadataResponse response = new SingleSourceControlMetadataResponse(app, lastRefreshTime); | ||
| responder.sendJson(HttpResponseStatus.OK, GSON.toJson(response)); | ||
| } | ||
|
|
||
| private ScanApplicationsRequest getScanRequest(String namespaceId, String artifactVersion, | ||
|
|
@@ -375,7 +377,7 @@ | |
| } | ||
| if (nameFilter != null && !nameFilter.isEmpty()) { | ||
| if (nameFilterType != null) { | ||
| switch (nameFilterType) { | ||
| case EQUALS: | ||
| builder.setApplicationReference(new ApplicationReference(namespaceId, nameFilter)); | ||
| break; | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why use boxed type? Is it nullable? It's better not to use box type if possible. Is there a meaning default value for it that you can use?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its null if the refresh has not been run yet. I think it would be more appropiate to have default value as null in the API response than a actual long value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it can ever be null. The value comes from
SourceControlMetadataRefresher.getLastRefreshTime, which returns primitive long, and the implementation is defaulting it to0L