Skip to content

Commit 3c5ad06

Browse files
committed
fix(bulk-model-sync-lib): use carriage returns only on TTYs
The implemented progress reporting in the ModelImporter always used carriage returns to report progress. This only works in terminals and breaks incremental logging in setups like k8s. This commit tries to detect if running in a terminal and if not, resorts to logging calls for progress reporting. Logging output is reduced to avoid excessive spam in captured log file.
1 parent 56655da commit 3c5ad06

File tree

5 files changed

+500
-10
lines changed

5 files changed

+500
-10
lines changed

bulk-model-sync-lib/src/commonMain/kotlin/org/modelix/model/sync/bulk/ModelImporter.kt

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -77,12 +77,13 @@ class ModelImporter(private val root: INode, private val continueOnError: Boolea
7777
postponedReferences.clear()
7878
nodesToRemove.clear()
7979
numExpectedNodes = countExpectedNodes(data.root)
80+
val progressReporter = ProgressReporter(numExpectedNodes.toULong(), logger)
8081
currentNodeProgress = 0
8182
buildExistingIndex(root)
8283

8384
logger.info { "Importing nodes..." }
8485
data.root.originalId()?.let { originalIdToExisting[it] = root }
85-
syncNode(root, data.root)
86+
syncNode(root, data.root, progressReporter)
8687

8788
logger.info { "Synchronizing references..." }
8889
postponedReferences.forEach {
@@ -107,18 +108,17 @@ class ModelImporter(private val root: INode, private val continueOnError: Boolea
107108
private fun countExpectedNodes(data: NodeData): Int =
108109
1 + data.children.sumOf { countExpectedNodes(it) }
109110

110-
private fun syncNode(node: INode, data: NodeData) {
111+
private fun syncNode(node: INode, data: NodeData, progressReporter: ProgressReporter) {
111112
currentNodeProgress += 1
112-
// print instead of log, so that the progress line can be overwritten by the carriage return
113-
print("\r($currentNodeProgress / $numExpectedNodes) Synchronizing nodes... ")
113+
progressReporter.step(currentNodeProgress.toULong())
114114
doAndPotentiallyContinueOnErrors {
115115
syncProperties(node, data)
116-
syncChildren(node, data)
116+
syncChildren(node, data, progressReporter)
117117
syncReferences(node, data)
118118
}
119119
}
120120

121-
private fun syncChildren(node: INode, data: NodeData) {
121+
private fun syncChildren(node: INode, data: NodeData, progressReporter: ProgressReporter) {
122122
val allRoles = (data.children.map { it.role } + node.allChildren.map { it.roleInParent }).distinct()
123123
for (role in allRoles) {
124124
val expectedNodes = data.children.filter { it.role == role }
@@ -130,15 +130,15 @@ class ModelImporter(private val root: INode, private val continueOnError: Boolea
130130
val expectedId = checkNotNull(expected.originalId()) { "Specified node '$expected' has no id" }
131131
newChild.setPropertyValue(NodeData.idPropertyKey, expectedId)
132132
originalIdToExisting[expectedId] = newChild
133-
syncNode(newChild, expected)
133+
syncNode(newChild, expected, progressReporter)
134134
}
135135
continue
136136
}
137137

138138
// optimization for when there is no change in the child list
139139
// size check first to avoid querying the original ID
140140
if (expectedNodes.size == existingNodes.size && expectedNodes.map { it.originalId() } == existingNodes.map { it.originalId() }) {
141-
existingNodes.zip(expectedNodes).forEach { syncNode(it.first, it.second) }
141+
existingNodes.zip(expectedNodes).forEach { syncNode(it.first, it.second, progressReporter) }
142142
continue
143143
}
144144

@@ -163,7 +163,7 @@ class ModelImporter(private val root: INode, private val continueOnError: Boolea
163163
}
164164
check(childNode.getConceptReference() == expectedConcept) { "Unexpected concept change" }
165165

166-
syncNode(childNode, expected)
166+
syncNode(childNode, expected, progressReporter)
167167
}
168168

169169
nodesToRemove += node.getChildren(role).drop(expectedNodes.size)

bulk-model-sync-lib/src/commonMain/kotlin/org/modelix/model/sync/bulk/Util.kt

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import mu.KLogger
2020
import org.modelix.model.api.BuiltinLanguages
2121
import org.modelix.model.data.ModelData
2222
import org.modelix.model.data.NodeData
23+
import kotlin.math.max
2324

2425
/**
2526
* Checks if a module is included in the sync.
@@ -28,7 +29,11 @@ import org.modelix.model.data.NodeData
2829
* @param includedModules collection of included module names
2930
* @param includedPrefixes collection of included module name prefixes
3031
*/
31-
fun isModuleIncluded(moduleName: String, includedModules: Collection<String>, includedPrefixes: Collection<String>): Boolean {
32+
fun isModuleIncluded(
33+
moduleName: String,
34+
includedModules: Collection<String>,
35+
includedPrefixes: Collection<String>,
36+
): Boolean {
3237
val includedDirectly = includedModules.contains(moduleName)
3338
val includedByPrefix = includedPrefixes.any { prefix -> moduleName.startsWith(prefix) }
3439

@@ -81,3 +86,33 @@ private fun measureImportSize(data: NodeData, metrics: ImportSizeMetrics = Impor
8186
data.children.forEach { measureImportSize(it, metrics) }
8287
return metrics
8388
}
89+
90+
expect fun isTty(): Boolean
91+
92+
class ProgressReporter(
93+
private val total: ULong,
94+
private val logger: KLogger,
95+
private val print: (line: String) -> Unit = ::println,
96+
private val isTty: () -> Boolean = ::isTty,
97+
) {
98+
99+
// Determine how often to log. For a small number of total nodes, ensure that we log at all. Otherwise,
100+
// update progress ever 1% of the total to avoid spamming the output with log lines.
101+
private val loggingStepSize = max(1UL, (total.toDouble() / 100).toULong())
102+
103+
/**
104+
* Increments the progress. This function is assumed to be called for every element that's processed.
105+
*/
106+
fun step(current: ULong) {
107+
if (isTty()) {
108+
// print instead of log, so that the progress line can be overwritten by the carriage return
109+
print("\r($current / $total) Synchronizing nodes... ")
110+
} else {
111+
// Report on desired increments or first and last element to ensure users get important start and end
112+
// information
113+
if ((current % loggingStepSize) == 0UL || current == total || current == 1UL) {
114+
logger.info { "($current / $total) Synchronizing nodes..." }
115+
}
116+
}
117+
}
118+
}
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
/*
2+
* Copyright (c) 2023.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.modelix.model.sync.bulk
18+
19+
actual fun isTty(): Boolean {
20+
// Be safe and never assume a terminal as we cannot easily test for a TTY being attached
21+
return false
22+
}
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/*
2+
* Copyright (c) 2023.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.modelix.model.sync.bulk
18+
19+
actual fun isTty(): Boolean {
20+
return System.console() != null
21+
}

0 commit comments

Comments
 (0)