[CDAP-20993] Periodic refresh implementation of source control metadata for SCM sync status #15626

adrikagupta · 2024-04-29T15:29:21Z

Added Periodic refresh for namespace and repository source control metadata
- Source control metadata is automatically refreshed every 3 hours which is configurable
- There is a way to manually refresh the source control metadata by calling the list APIs
- If the time elapsed since the last refresh is within the buffer time of 10 minutes (this is configurable), then the manual trigger is initiated
  - After setting a repo config, manual trigger is initiated. The above check is skipped.
- If the service for that namespace is already running, refresh operation is skipped.

cdap-common/src/main/resources/cdap-default.xml

cdap-features/src/main/java/io/cdap/cdap/features/Feature.java

samdgupi · 2024-05-03T05:25:53Z

cdap-app-fabric/src/main/java/io/cdap/cdap/app/store/SyncStatus.java

+ * It is used in filtering.
+ */
+public enum SyncStatus {
+ SYNCED,


delete this

samdgupi · 2024-05-03T05:26:43Z

cdap-app-fabric/src/main/java/io/cdap/cdap/gateway/handlers/AppLifecycleHttpHandler.java

+ SourceControlMetadataRecord record = lastRecord.get();
+ String nextPageToken = !limitReachedAndLastRefreshPair.getKey() || record == null ? null :
+ record.getName();
+ Long lastRefreshTime = limitReachedAndLastRefreshPair.getValue();


just expose a function in applicationLifecycleService to get the last refresh time

...p-fabric/src/main/java/io/cdap/cdap/gateway/handlers/SourceControlManagementHttpHandler.java

...c/main/java/io/cdap/cdap/internal/app/sourcecontrol/SourceControlMetadataRefreshService.java

samdgupi · 2024-05-03T06:03:32Z

...c/main/java/io/cdap/cdap/internal/app/sourcecontrol/SourceControlMetadataRefreshService.java

+
+ public void removeRefreshService(NamespaceId namespaceId) {
+ if(refreshSchedulers.containsKey(namespaceId)) {
+ refreshSchedulers.get(namespaceId).stop();


check if there is some Optional return

cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/store/DefaultStore.java

samdgupi · 2024-05-03T06:12:26Z

cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/store/DefaultStore.java

 AppMetadataStore appMetadataStore = getAppMetadataStore(context);
 for (Map.Entry<ApplicationId, SourceControlMeta> updateRequest : updateRequests.entrySet()) {
 ApplicationId appId = updateRequest.getKey();
 if (appMetadataStore.getApplication(appId) != null) {
 sourceControlMetadataStore.write(appId.getAppReference(),
 SourceControlMeta.builder(updateRequest.getValue()).setSyncStatus(true).build());
+ if (repoStore.get(appId.getAppReference()) != null) {


you should always update repostore

samdgupi · 2024-05-03T06:12:35Z

cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/store/DefaultStore.java

 TransactionRunners.run(transactionRunner, context -> {
 getNamespaceSourceControlMetadataStore(context).write(appRef,
 SourceControlMeta.builder(sourceControlMeta).setSyncStatus(true).build());
+ RepositorySourceControlMetadataStore repoStore = getRepoSourceControlMetadataStore(context);
+ if (repoStore.get(appRef) != null) {


same as above

samdgupi · 2024-05-06T04:48:06Z

cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/store/DefaultStore.java

 getNamespaceSourceControlMetadataStore(context).write(id.getAppReference(),
 meta.getSourceControlMeta() == null ? SourceControlMeta.createDefaultMeta()
 : SourceControlMeta.builder(meta.getSourceControlMeta()).setSyncStatus(true).build());
+ if (repoStore.get(appRef) != null) {
+ repoStore.write(appRef, meta.getSourceControlMeta() != null,


nit: Please add temporary variables to hold isSync, LastModified value. This way it would be easy to understand what is being passed to write()

samdgupi · 2024-05-06T04:50:11Z

...c/main/java/io/cdap/cdap/internal/app/sourcecontrol/SourceControlMetadataRefreshService.java

+ + namespaceId.getNamespace());
+ refreshSchedulers.get(namespaceId).triggerManualRefresh(false);
+ } else {
+ addRefreshService(namespaceId);


this should always happen before the if

But addRefreshService should be called only if it is not already present in refreshSchedulers, right?

addRefreshService should have putIfAbsent that would take care of existing scheduler issue

...c/main/java/io/cdap/cdap/internal/app/sourcecontrol/SourceControlMetadataRefreshService.java

samdgupi · 2024-05-06T04:52:27Z

...c/main/java/io/cdap/cdap/internal/app/sourcecontrol/SourceControlMetadataRefreshService.java

+ * @param namespaceId The ID of the namespace for which to add the refresh service.
+ */
+ public void addRefreshService(NamespaceId namespaceId) {
+ LOG.info("Adding refresh service for " + namespaceId.getNamespace());


debug log. and add it after you added the scheduler

samdgupi · 2024-05-06T04:52:54Z

...c/main/java/io/cdap/cdap/internal/app/sourcecontrol/SourceControlMetadataRefreshService.java

+ */
+ public void addRefreshService(NamespaceId namespaceId) {
+ LOG.info("Adding refresh service for " + namespaceId.getNamespace());
+ if (!refreshSchedulers.containsKey(namespaceId)) {


use putIfNotPresent. The current logic is not threadsafe

samdgupi · 2024-05-06T04:54:09Z

...c/main/java/io/cdap/cdap/internal/app/sourcecontrol/SourceControlMetadataRefreshService.java

+ LOG.info("Shutting down SourceControlManagementService");
+
+ for (NamespaceSourceControlMetadataRefreshService service : refreshSchedulers.values()) {
+ service.stop();


stopandwait()

samdgupi · 2024-05-06T04:55:24Z

...c/main/java/io/cdap/cdap/internal/app/sourcecontrol/SourceControlMetadataRefreshService.java

+ LOG.info("Starting SourceControlMetadataRefreshManager");
+
+ List<NamespaceId> namespaceIds = namespaceAdmin.list().stream()
+ .map(meta -> meta.getNamespaceId()).collect(Collectors.toList());


you can do

if (getRepositoryMeta(namespaceId) != null) { addRefreshService(namespaceId); }

here only. no need for a list

samdgupi · 2024-05-06T04:56:39Z

...app-fabric/src/main/java/io/cdap/cdap/internal/app/services/ApplicationLifecycleService.java

+
+ // Triggering source control metadata refresh service
+ try {
+ checkSourceControlMetadataAutoRefreshFlag();


Please do not do this. Otherwise in case if the flag is false the API would not work at all. Just skip refresh if the flag is not enabled

samdgupi · 2024-05-06T04:58:11Z

cdap-app-fabric/src/main/java/io/cdap/cdap/app/store/SingleSourceControlMetadataResponse.java

+
+ public SingleSourceControlMetadataResponse(SourceControlMetadataRecord record,
+ Long lastRefreshTime) {
+ this.record = record;


dont name it record. rather name it app. Similar to how list api is calling this apps

samdgupi · 2024-05-06T05:03:04Z

...in/java/io/cdap/cdap/internal/app/services/NamespaceSourceControlMetadataRefreshService.java

+ @Override
+ public void runOneIteration() {
+
+ running.set(true);


Please call runrefreshservice here

if (!running.compareAndSet(false, true)) { return; } runRefreshService(forced);

samdgupi · 2024-05-06T14:14:23Z

cdap-common/src/main/resources/cdap-default.xml

+ </property>
+ <property>
+ <name>source.control.metadata.refresh.buffer.seconds</name>
+<!-- <value>600</value>-->


Please fix. Set it to 10 min by default

samdgupi · 2024-05-06T14:15:36Z

...bric/src/main/java/io/cdap/cdap/internal/app/store/RepositorySourceControlMetadataStore.java

Please fix checkstyle

samdgupi · 2024-05-06T14:16:08Z

...ava/io/cdap/cdap/internal/app/services/NamespaceSourceControlMetadataRefreshServiceTest.java

Please fix checkstyle

samdgupi · 2024-05-06T14:19:09Z

cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/store/DefaultStore.java

+ meta.getSourceControlMeta() == null ? SourceControlMeta.createDefaultMeta()
+ : SourceControlMeta.builder(meta.getSourceControlMeta()).setSyncStatus(true)
+ .build());
+ if (repoStore.get(appRef) != null) {


Should we not also write when meta.getSourceControlMeta() != null ?

samdgupi · 2024-05-06T14:19:34Z

cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/store/DefaultStore.java

 getNamespaceSourceControlMetadataStore(context).write(id.getAppReference(),
 meta.getSourceControlMeta() == null ? SourceControlMeta.createDefaultMeta()
 : SourceControlMeta.builder(meta.getSourceControlMeta()).setSyncStatus(true).build());
+ if (repoStore.get(appRef) != null) {


Should we not also write when meta.getSourceControlMeta() != null ?

Sure, I have changed to -

if (meta.getSourceControlMeta() != null) { Instant lastSyncedAt = meta.getSourceControlMeta().getLastSyncedAt(); repoStore.write(appRef, true, lastSyncedAt == null ? null : lastSyncedAt.toEpochMilli()); } else{ ImmutablePair<Long, Boolean> lastSyncedAndStatusPair = repoStore.get(appRef); if(lastSyncedAndStatusPair != null) { repoStore.write(appRef, false, lastSyncedAndStatusPair.getFirst()); } }

Three cases should be considered:

If we are deploying an app in namespace and hasn't been pulled or pushed before - In this case we check, scm meta is null then we dont write anything in repo table since it is not present in the repo table.

If we are deploying an app already been pulled/pushed before - In this case the record will be present in repo table hence we will change the sync status but retain the last modified.

If we are pulling and deploying - In this case since scm meta will not be null we will be setting the values in repo table.

I feel the previous implementation might have also worked based on the concept that if we are pulling and deploying or are deploying an app which has been pulled/pushed before, the record would be there in the repo table.

...c/main/java/io/cdap/cdap/internal/app/sourcecontrol/SourceControlMetadataRefreshService.java

samdgupi · 2024-05-06T14:21:57Z

...c/main/java/io/cdap/cdap/internal/app/sourcecontrol/SourceControlMetadataRefreshService.java

+ public void addRefreshService(NamespaceId namespaceId) {
+ NamespaceSourceControlMetadataRefreshService service =
+ refreshSchedulers.putIfAbsent(namespaceId, createRefreshService(namespaceId));
+ if (service == null || isSourceControlMetadataPeriodicRefreshFlagEnabled()) {


add the service only if flag is enabled

But it is possible that the user disables the periodic refresh flag but keeps manual refresh flag enabled

samdgupi · 2024-05-06T14:22:18Z

...c/main/java/io/cdap/cdap/internal/app/sourcecontrol/SourceControlMetadataRefreshService.java

+ refreshSchedulers.get(namespaceId).start();
+ LOG.debug("Added Scheduled NamespaceSourceControlMetadataRefreshService for "
+ + namespaceId.getNamespace());
+ } else {


why are we triggering here again ?

public void addRefreshService(NamespaceId namespaceId) { NamespaceSourceControlMetadataRefreshService service = refreshSchedulers.putIfAbsent(namespaceId, createRefreshService(namespaceId)); if (service == null || isSourceControlMetadataPeriodicRefreshFlagEnabled()) { refreshSchedulers.get(namespaceId).start(); } else { refreshSchedulers.get(namespaceId).triggerManualRefresh(true); } }

If we are setting the repo config for the first time, it creates refresh service, calls start function, and performs runOneIteration.
When we are updating the repo config, it is already present in refreshSchedulers. If start() function is called, it will see that it has already started and will not call runOneIteration instantly. So we have to trigger it manually so that the changes are reflected in the git pipelines page.

...c/main/java/io/cdap/cdap/internal/app/sourcecontrol/SourceControlMetadataRefreshService.java

samdgupi · 2024-05-06T14:24:04Z

...in/java/io/cdap/cdap/internal/app/services/NamespaceSourceControlMetadataRefreshService.java

+ protected void startUp() throws Exception {
+ // This gives users an option to disable the source control metadata refresh service
+ // for all namespaces
+ if (this.runInterval <= 0) {


remove this as we already have a feature flag

samdgupi

Merge only after requested changes are done

samdgupi · 2024-05-06T19:01:33Z

...bric/src/main/java/io/cdap/cdap/internal/app/store/RepositorySourceControlMetadataStore.java

+ * or null if metadata is not found.
+ * @throws IOException If an I/O error occurs.
+ */
+ public ImmutablePair get(ApplicationReference appRef) throws IOException {


why is this changed needed ? why we cant use SourceControlMetadataRecord

samdgupi · 2024-05-06T19:03:22Z

cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/store/DefaultStore.java

+ lastSyncedAt.toEpochMilli());
+ }
+ else{
+ ImmutablePair<Long, Boolean> lastSyncedAndStatusPair = repoStore.get(appRef);


Why cant we just use SourceControlMetadataRecord ?

samdgupi · 2024-05-06T19:03:54Z

cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/store/DefaultStore.java

+ else{
+ ImmutablePair<Long, Boolean> lastSyncedAndStatusPair = repoStore.get(appRef);
+ if(lastSyncedAndStatusPair != null) {
+ repoStore.write(appRef, false, lastSyncedAndStatusPair.getFirst());


As mentioned before please add a separate function to toggle sync status only

samdgupi · 2024-05-06T19:08:57Z

...c/main/java/io/cdap/cdap/internal/app/sourcecontrol/SourceControlMetadataRefreshService.java

+ */
+ public Long getLastRefreshTime(NamespaceId namespaceId) {
+ if (refreshSchedulers.containsKey(namespaceId)) {
+ return refreshSchedulers.get(namespaceId).getLastRefreshTime();


This is not correct as refreshSchedulers.get(namespaceId) can be null. Rather get refreshSchedulers.get(namespaceId) in a temp variable and check if it is null

samdgupi · 2024-05-06T19:11:11Z

...c/main/java/io/cdap/cdap/internal/app/sourcecontrol/SourceControlMetadataRefreshService.java

+ protected void shutDown() throws Exception {
+ LOG.info("Shutting down SourceControlMetadataRefreshService");
+
+ for (NamespaceSourceControlMetadataRefreshService service : refreshSchedulers.values()) {


nit: Use forEach

chtyim · 2024-05-06T19:57:39Z

cdap-app-fabric/src/main/java/io/cdap/cdap/gateway/handlers/AppLifecycleHttpHandler.java

+ lastRecord.set(scmMetaRecord);
+ });
+ } catch (IOException e) {
+ responder.sendString(HttpResponseStatus.INTERNAL_SERVER_ERROR, e.getMessage());


Please throw exception and let the http exception handler to handle the response. If you send here, you need to return from the catch block, otherwise the flow will continue.

chtyim · 2024-05-06T20:00:01Z

cdap-app-fabric/src/main/java/io/cdap/cdap/gateway/handlers/AppLifecycleHttpHandler.java

+ } catch (IOException e) {
+ responder.sendString(HttpResponseStatus.INTERNAL_SERVER_ERROR, e.getMessage());
+ }
+ SourceControlMetadataRecord record = lastRecord.get();


The lastRecord variable seems unnecessary. The same can be achieved by apps.isEmpty() and apps.get(apps.size() - 1).

chtyim · 2024-05-06T20:02:19Z

cdap-app-fabric/src/main/java/io/cdap/cdap/gateway/handlers/AppLifecycleHttpHandler.java

+ SourceControlMetadataRecord record = lastRecord.get();
+ String nextPageToken = !pageLimitReached || record == null ? null :
+ record.getName();
+ Long lastRefreshTime = applicationLifecycleService.getLastRefreshTime(namespaceId);


This would execute another SQL in another transaction, which may be inconsistent with what has been returned by the scanSourceControlMetadata call. I think it's better for the scan method to return a response that contains apps, page limit, and last scan time. This makes sure all the data are coming from the same transaction.

chtyim · 2024-05-06T20:03:25Z

...p-fabric/src/main/java/io/cdap/cdap/gateway/handlers/SourceControlManagementHttpHandler.java

+ lastRecord.set(record);
+ });
+ } catch (IOException e) {
+ responder.sendString(HttpResponseStatus.INTERNAL_SERVER_ERROR, e.getMessage());


Same as above. Please throw. The old code was wrong too, please don't follow with that pattern.

chtyim · 2024-05-06T20:07:30Z

...p-fabric/src/main/java/io/cdap/cdap/gateway/handlers/SourceControlManagementHttpHandler.java

+ SourceControlMetadataRecord record = lastRecord.get();
+ String nextPageToken = !pageLimitReached || record == null ? null :
+ record.getName();
+ Long lastRefreshTime = sourceControlService.getLastRefreshTime(namespaceId);


Should be using primitive long instead.

chtyim · 2024-05-06T20:08:34Z

...app-fabric/src/main/java/io/cdap/cdap/internal/app/services/ApplicationLifecycleService.java

+
+ // Triggering source control metadata refresh service
+ if (isSourceControlMetadataManualRefreshFlagEnabled()) {
+ sourceControlMetadataRefreshService.runRefreshService(namespaceId);


Why the refreshment is enabled only when there is a scan?

chtyim · 2024-05-06T20:10:44Z

...in/java/io/cdap/cdap/internal/app/services/NamespaceSourceControlMetadataRefreshService.java

+
+ @Override
+ protected final ScheduledExecutorService executor() {
+ executor = Executors.newSingleThreadScheduledExecutor(


So we have one thread per namespace for the refresh? Seems not an efficient way of using thread. Ideally we only need a small scheduled thread pool for all namespaces refresh?

chtyim · 2024-05-06T20:18:02Z

...in/java/io/cdap/cdap/internal/app/services/NamespaceSourceControlMetadataRefreshService.java

+ // TODO(CDAP-21017): Optimize periodic refresh of source control metadata
+ @Override
+ public void runOneIteration() {
+ if (!running.compareAndSet(false, true)) {


You shouldn't need an atomic boolean for this. The AbstractScheduledService should have the correct guarantee of how the scheduled run is.

chtyim · 2024-05-06T20:18:29Z

...in/java/io/cdap/cdap/internal/app/services/NamespaceSourceControlMetadataRefreshService.java

+
+ private void runRefreshService(boolean forced) {
+ running.set(true);
+ Long refreshStartTime = System.currentTimeMillis();


Use primitive long.

chtyim · 2024-05-06T20:19:08Z

...in/java/io/cdap/cdap/internal/app/services/NamespaceSourceControlMetadataRefreshService.java

+
+ try {
+
+ if (!namespaceAdmin.exists(namespaceId)) {


Everything that happens in one run should happens in the same transaction. Using admin will create a different transaction boundary.

chtyim · 2024-05-06T20:19:56Z

...in/java/io/cdap/cdap/internal/app/services/NamespaceSourceControlMetadataRefreshService.java

+ new NamespaceRepository(namespaceId, repoConfig));
+
+ // Cleaning up the repo source control metadata table
+ HashSet<String> repoFileNames = repositoryAppsResponse.getApps().stream()


Use interface Set as the variable type.

chtyim · 2024-05-06T20:20:43Z

...in/java/io/cdap/cdap/internal/app/services/NamespaceSourceControlMetadataRefreshService.java

+ lastRefreshTime.set(refreshStartTime);
+
+ } catch (Exception e) {
+ LOG.error("Failed to refresh source control metadata for namespace "


Use placeholder {} for logging instead of string concat. Also, seems like a warning than error.

chtyim · 2024-05-06T20:21:43Z

...in/java/io/cdap/cdap/internal/app/services/NamespaceSourceControlMetadataRefreshService.java

+ @Override
+ protected void startUp() throws Exception {
+ LOG.info("Starting SourceControlMetadataRefreshService for namespace "
+ + namespaceId.getNamespace() + " with interval " + runInterval + " seconds");


Use placeholder {} instead of string concat for loggin.

chtyim · 2024-05-06T20:22:10Z

...in/java/io/cdap/cdap/internal/app/services/NamespaceSourceControlMetadataRefreshService.java

+ }
+
+ private void updateSourceControlMeta(ApplicationReference appRef, String repoFileHash,
+ Long refreshStartTime) {


Should use primitive long.

chtyim · 2024-05-06T20:22:40Z

...in/java/io/cdap/cdap/internal/app/services/NamespaceSourceControlMetadataRefreshService.java

+ .setNamespace(namespace)
+ .setLimit(Integer.MAX_VALUE)
+ .build();
+ ArrayList<SourceControlMetadataRecord> records = new ArrayList<>();


Use List as the type.

chtyim · 2024-05-06T20:24:40Z

...in/java/io/cdap/cdap/internal/app/services/NamespaceSourceControlMetadataRefreshService.java

+ throws IOException {
+ for (SourceControlMetadataRecord record : records) {
+ if (!repoFiles.contains(record.getName())) {
+ store.delete(new ApplicationReference(record.getNamespace(), record.getName()));


What does the store.delete actually do? Asking it because this method is nested inside the transactional scan method.

chtyim · 2024-05-06T20:25:03Z

...-fabric/src/main/java/io/cdap/cdap/internal/app/services/SourceControlManagementService.java

 return true;
 }

+ public Long getLastRefreshTime(String namespace) {


Should use primitive long.

chtyim

Please address the top level comments first. I will take another pass after those are fixed.

samdgupi force-pushed the refresh-sync branch from c6c06ec to c742f77 Compare May 1, 2024 09:38

adrikagupta force-pushed the refresh-sync branch 2 times, most recently from eb92835 to d8b506f Compare May 2, 2024 22:36

GnsP added build Triggers github actions build 6.10.1 labels May 3, 2024

adrikagupta force-pushed the refresh-sync branch from d8b506f to 49e5248 Compare May 3, 2024 04:17

samdgupi reviewed May 3, 2024

View reviewed changes

adrikagupta added build Triggers github actions build and removed 6.10.1 build Triggers github actions build labels May 4, 2024

adrikagupta force-pushed the refresh-sync branch from 49e5248 to 507b5f8 Compare May 5, 2024 22:40

adrikagupta added the 6.10.1 label May 5, 2024

samdgupi reviewed May 6, 2024

View reviewed changes

adrikagupta force-pushed the refresh-sync branch from 507b5f8 to d546252 Compare May 6, 2024 12:58

samdgupi reviewed May 6, 2024

View reviewed changes

adrikagupta force-pushed the refresh-sync branch from d546252 to 382f9d1 Compare May 6, 2024 18:42

samdgupi approved these changes May 6, 2024

View reviewed changes

adrikagupta force-pushed the refresh-sync branch from 382f9d1 to 20aaa5b Compare May 6, 2024 19:47

added implementation of peridic refresh for source control metadata

17956c8

adrikagupta force-pushed the refresh-sync branch from 20aaa5b to 17956c8 Compare May 6, 2024 19:55

chtyim reviewed May 6, 2024

View reviewed changes

chtyim requested changes May 6, 2024

View reviewed changes

[CDAP-20993] Periodic refresh implementation of source control metadata for SCM sync status #15626

Are you sure you want to change the base?

[CDAP-20993] Periodic refresh implementation of source control metadata for SCM sync status #15626

Conversation

adrikagupta commented Apr 29, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adrikagupta May 6, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samdgupi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chtyim left a comment

Choose a reason for hiding this comment

adrikagupta commented Apr 29, 2024 •

edited

adrikagupta May 6, 2024 •

edited