Spark 3.4+ support

https://spark.apache.org/news/spark-3-4-0-released.html

Hi. I've just started looking at Spark 3.4 and the first issue we ran into was that we're missing this library. For sure it would be a big win if we could support it.

Please vote top comment if you need!

I am very interested in keeping this alive as well.

What would be the next step towards moving forward with this?

What would be the next step towards moving forward with this?

The next step would be to investigate to find a new way to encode Kotlin Data Classes (both at the top-level of DataFrames, as well as inside columns) and keep inferring types to encoders without using KotlinReflection.scala, such that it's compatible with all versions of Scala and Spark 3.X by default. That way we can keep the API relatively the same and improve the maintainability, compatibility, and stability.
I'm not sure which mechanisms of Spark we can leverage for this; I was thinking of maybe using UDTs and a compiler plugin/annotation processor to generate the UDT classes... but that won't work for top-level tables.
Unfortunately, I'm too occupied with Kotlin DataFrame at the moment, but if someone could provide a proof of concept I'm sure I can provide some help :).

So if I understood that correctly, we will be able to create spark Dataset from a kotlinx DataFrame? That's exactly what I wanted to do, because working with spark Datasets is not that smooth.

So if I understood that correctly, we will be able to create spark Dataset from a kotlinx DataFrame? That's exactly what I wanted to do, because working with spark Datasets is not that smooth.
Btw. is there currently a workaround for this?

No, that's currently not on the roadmap. They're two separate projects, although, we are exploring interop with other databases in DataFrame (https://github.com/Kotlin/dataframe/issues/408) (including Spark).
If you want to convert from Kotlin DataFrame to Spark DataSets, that's actually quite simple:
@DataSchema
data class Name(
    val firstName: String,
    val lastName: String,
)

@DataSchema
data class Person(
    val name: Name,
    val age: Int,
    val city: String?,
    val weight: Int?,
    val isHappy: Boolean,
)

// Kotlin DataFrame
val df: DataFrame<Person> = listOf(
    Person(Name("Alice", "Cooper"), 15, "London", 54, true),
    Person(Name("Bob", "Dylan"), 45, "Dubai", 87, true),
    Person(Name("Charlie", "Daniels"), 20, "Moscow", null, false),
    Person(Name("Charlie", "Chaplin"), 40, "Milan", null, true),
    Person(Name("Bob", "Marley"), 30, "Tokyo", 68, true),
    Person(Name("Alice", "Wolf"), 20, null, 55, false),
    Person(Name("Charlie", "Byrd"), 30, "Moscow", 90, true),
).toDataFrame()

withSpark {
    // Spark Dataset
    val sparkDs: DataSet<Person> = df.toList().toDS()
}

Note that df.toList()/df.toListOf<>() only works if the return type is a data class, which is also what's needed for Spark.
If you want to be able to convert any Kotlin DataFrame to a Spark Dataset<Row>, we'll need to convert the schema as well:
/**
 * Converts the DataFrame to a Spark Dataset of Rows using the provided SparkSession and JavaSparkContext.
 *
 * @param spark The SparkSession object to use for creating the DataFrame.
 * @param sc The JavaSparkContext object to use for converting the DataFrame to RDD.
 * @return A Dataset of Rows representing the converted DataFrame.
 */
fun DataFrame<*>.toSpark(spark: SparkSession, sc: JavaSparkContext): Dataset<Row> {
    val rows = sc.toRDD(rows().map(DataRow<*>::toSpark))
    return spark.createDataFrame(rows, schema().toSpark())
}

/**
 * Converts a DataRow to a Spark Row object.
 *
 * @return The converted Spark Row.
 */
fun DataRow<*>.toSpark(): Row =
    RowFactory.create(
        *values().map {
            when (it) {
                is DataRow<*> -> it.toSpark()
                else -> it
            }
        }.toTypedArray()
    )

/**
 * Converts a DataFrameSchema to a Spark StructType.
 *
 * @return The converted Spark StructType.
 */
fun DataFrameSchema.toSpark(): StructType =
    DataTypes.createStructType(
        columns.map { (name, schema) ->
            DataTypes.createStructField(name, schema.toSpark(), schema.nullable)
        }
    )

/**
 * Converts a ColumnSchema object to Spark DataType.
 *
 * @return The Spark DataType corresponding to the given ColumnSchema object.
 * @throws IllegalArgumentException if the column type or kind is unknown.
 */
fun ColumnSchema.toSpark(): DataType =
    when (this) {
        is ColumnSchema.Value -> type.toSpark() ?: error("unknown data type: $type")
        is ColumnSchema.Group -> schema.toSpark()
        is ColumnSchema.Frame -> error("nested dataframes are not supported")
        else -> error("unknown column kind: $this")
    }

/**
 * Returns the corresponding Spark DataType for a given Kotlin type.
 *
 * @return The Spark DataType that corresponds to the Kotlin type, or null if no matching DataType is found.
 */
fun KType.toSpark(): DataType? = when(this) {
    typeOf<Byte>(), typeOf<Byte?>() -> DataTypes.ByteType
    typeOf<Short>(), typeOf<Short?>() -> DataTypes.ShortType
    typeOf<Int>(), typeOf<Int?>() -> DataTypes.IntegerType
    typeOf<Long>(), typeOf<Long?>() -> DataTypes.LongType
    typeOf<Boolean>(), typeOf<Boolean?>() -> DataTypes.BooleanType
    typeOf<Float>(), typeOf<Float?>() -> DataTypes.FloatType
    typeOf<Double>(), typeOf<Double?>() -> DataTypes.DoubleType
    typeOf<String>(), typeOf<String?>() -> DataTypes.StringType
    typeOf<LocalDate>(), typeOf<LocalDate?>() -> DataTypes.DateType
    typeOf<Date>(), typeOf<Date?>() -> DataTypes.DateType
    typeOf<Timestamp>(), typeOf<Timestamp?>() -> DataTypes.TimestampType
    typeOf<Instant>(), typeOf<Instant?>() -> DataTypes.TimestampType
    typeOf<ByteArray>(), typeOf<ByteArray?>() -> DataTypes.BinaryType
    typeOf<Decimal>(), typeOf<Decimal?>() -> DecimalType.SYSTEM_DEFAULT()
    typeOf<BigDecimal>(), typeOf<BigDecimal?>() -> DecimalType.SYSTEM_DEFAULT()
    typeOf<BigInteger>(), typeOf<BigInteger?>() -> DecimalType.SYSTEM_DEFAULT()
    typeOf<CalendarInterval>(), typeOf<CalendarInterval?>() -> DataTypes.CalendarIntervalType
    else -> null
}

withSpark {
    // Spark Dataset
    val sparkDs: DataSet<Row> = df.toSpark(spark, sc)
}

@Jolanrensen Can you explain a little bit more about what ScalaReflection.scala and KotlinReflection.scala do, what they're for, and why the latter is a blocker to Spark 3.4 support? And what some of the considerations about using UDTs as a replacement might be?
Thank you!

Sure! But I gotta give a warning. I'm not the original author of the patch, just the maintainer of the rest of the library, so this will be my best understanding of what's going on.
One of the biggest features of the Kotlin Spark API is the automatic recognizing and encoding of types as Datasets. Without this, encoders would need to be given explicitly, as is the case for the Java API of Spark. This is the difference between:
spark.createDataset(listOf(1, 2, 3), Encoders.INT())
// and
listOf(1, 2, 3, 4, 5).toDS()

or even
data class Person1 @JvmOverloads constructor(
    var name: String? = null,
    var age: Int? = null,
) : Serializable

spark.createDataset(listOf(Person1("A", 1)), Encoders.bean(Person1::javaClass))

// and
data class Person2(
    val name: String,
    val age: Int,
)
listOf(Person2("A", 1)).toDS()

To do this, we need to automatically generate an encoder based on the typing information provided by the reified type parameter in toDS<>()  and the encoder<>() function.
If you follow the generateEncoder function in the same file you can see we'll attempt to create an Encoder from the given KType. This can be either a predefined encoder (from ENCODERS) or some data class, Map, iterable, Array, UDT or anything we can generate an Encoder for on the fly.
An (Expression)Encoder needs two things: An Expression to serialize an object and one to deserialize it. Functions to create these serializers and deserializers exist in ScalaReflection.scala, but they of course can only handle types supported by Spark itself. We want to be able to create (de)serializers for Kotlin Data classes, plus Tuples inside data classes, arrays inside tuples inside data classes inside maps, etc. We need the logic in this file but somehow inject extra functionality. At least, that's what @asm0dey likely thought when making it :). The downside is that we keep bumping into spark-internal functions if we want to call it from the Kotlin side (not even starting about the incompatibilities between Kotlin and Scala). So, a new module was created using the same org.apache.spark.sql package (to be able to call Spark internal functions) and the code from ScalaReflection.scala was copied to KotlinReflection.scala modifying its behavior for Kotlin data class support.
To help with the de(serializing) of Kotlin-specific stuff, a schema/predefinedDt argument was added such that the schema: DataType of a certain type can be generated in Kotlin instead of Scala.
Well, this worked, but, having a large piece of copied internal code in your codebase is bound to cause some issues over time. And so it did...
After each major release of Spark it was a large hassle to keep compatibility between KotlinReflection.scala and the rest of Spark. Especially, since internal calls can change between minor releases and break on a bytecode level. This is why we have so many releases (one for each Scala12/13 and minor Spark combo). Plus, if Spark adds a new feature to 3.Z, well, they can just update their ScalaReflection.scala file. We on the other hand need to support 3.X, 3.Y, ánd 3.Z with just one codebase (which we currently do with a preprocessor, but it's not a walk in the park).
Spark 3.4 was the straw that broke the camel's back. ScalaReflection.scala changed file location and a lot was changed between the last version. It was a good wake-up call to show that this wasn't the way forward. A simple preprocessor cannot ensure compatibility between these versions anymore and who knows what else will break in Spark 3.5 or with Scala 3 even.
We need a new way to encode Kotlin Data Classes while maintaining the current flexibility but without relying on internal Spark code. Spark version bumps (even major ones) need to be doable with minor preprocessor changes. (One version of the API for ALL spark versions is unrealistic, but one for, say 3.0, 3.1, etc. will probably be fine)
There are probably several ways to do this:

UDT: Make data classes automatically a user-defined-type, either with an @Annotation or using a Kotlin (2.0) compiler plugin. Downside: UDTs (afaik) only allow user defined objects serialized inside others, such as Tuples, not as a top-level table-like object. This could be possible, but I don't know how. Second, I don't know how (de)serializing of other known JVM types (such as tuples, Seqs, arrays etc) inside data classes would work.
Java Bean: Make a compiler plugin that will convert all data classes to something like  Person1 at compile time. Top-level should now work, but again I'm not sure about nested types.
Other clever Kotlinx reflection + Spark magic: Somehow be able to create an Encoder for any data class using reflection without the use of Spark internals or a compiler plugin. This would be the holy grail, but again, no clue how to do that.
Maybe even another way. I'm no Spark expert by any means. Maybe this gitbook could provide any inspiration.

Hopefully, this has given you or anyone interested enough inspiration and explanation to give it a try :) If someone can provide a proof-of-concept, I'd be happy to explore it further.

I'm happy to provide any additional guidance if needed as an author of the
original implementation (but probably Jolan knows it better than me already
:)
On Tue, 19 Sept 2023, 20:13 Jolan Rensen, @.***> wrote:

@Jolanrensen https://github.com/Jolanrensen Can you explain a little
bit more about what ScalaReflection.scala and KotlinReflection.scala do,
what they're for, and why the latter is a blocker to Spark 3.4 support? And
what some of the considerations about using UDTs as a replacement might be?
Thank you!
Sure! But I gotta give a warning. I'm not the original author of the
patch, just the maintainer of the rest of the library, so this will be my
best understanding of what's going on.
One of the biggest features of the Kotlin Spark API is the automatic
recognizing and encoding of types as Datasets. Without this, encoders would
need to be given explicitly, as is the case for the Java API of Spark
https://spark.apache.org/docs/latest/sql-getting-started.html#creating-datasets.
This is the difference between:
spark.createDataset(listOf(1, 2, 3), Encoders.INT())// andlistOf(1, 2, 3, 4, 5).toDS()
or even
data class Person1 @JvmOverloads constructor(
var name: String? = null,
var age: Int? = null,
) : Serializable
spark.createDataset(listOf(Person1("A", 1)), Encoders.bean(Person1::javaClass))
// anddata class Person2(
val name: String,
val age: Int,
)listOf(Person2("A", 1)).toDS()
To do this, we need to automatically generate an encoder based on the
typing information provided by the reified type parameter in toDS<>()
https://github.com/Kotlin/kotlin-spark-api/blob/470bcf4dd6a0318a1cd0e947670f921f8f62969e/kotlin-spark-api/src/main/kotlin/org/jetbrains/kotlinx/spark/api/SparkSession.kt#L64
and the encoder<>()
https://github.com/Kotlin/kotlin-spark-api/blob/470bcf4dd6a0318a1cd0e947670f921f8f62969e/kotlin-spark-api/src/main/kotlin/org/jetbrains/kotlinx/spark/api/Encoding.kt#L137
function.
If you follow the generateEncoder function in the same file you can see
we'll attempt to create an Encoder from the given KType. This can be
either a predefined encoder (from ENCODERS
https://github.com/Kotlin/kotlin-spark-api/blob/470bcf4dd6a0318a1cd0e947670f921f8f62969e/kotlin-spark-api/src/main/kotlin/org/jetbrains/kotlinx/spark/api/Encoding.kt#L78)
or some data class, Map, iterable, Array, UDT or anything we can generate
an Encoder for on the fly.
An (Expression)Encoder needs two things: An Expression to serialize an
object and one to deserialize it. Functions to create these serializers and
deserializers exist in ScalaReflection.scala
https://github.com/apache/spark/blob/2f3e4e36017d16d67086fd4ecaf39636a2fb4b7c/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/ScalaReflection.scala#L166C4-L166C4,
but they of course can only handle types supported by Spark itself. We want
to be able to create (de)serializers for Kotlin Data classes, plus Tuples
inside data classes, arrays inside tuples inside data classes inside maps,
etc. We need the logic in this file but somehow inject extra functionality.
At least, that's what @asm0dey https://github.com/asm0dey likely
thought when making it :). The downside is that we keep bumping into
spark-internal functions if we want to call it from the Kotlin side (not
even starting about the incompatibilities between Kotlin and Scala). So, a new
module
https://github.com/Kotlin/kotlin-spark-api/tree/release/core/src/main/scala
was created using the same org.apache.spark.sql package (to be able to
call Spark internal functions) and the code from ScalaReflection.scala
was copied to KotlinReflection.scala
https://github.com/Kotlin/kotlin-spark-api/blob/release/core/src/main/scala/org/apache/spark/sql/KotlinReflection.scala
modifying its behavior for Kotlin data class support.
To help with the de(serializing) of Kotlin-specific stuff, a schema/
predefinedDt argument was added such that the schema: DataType of a
certain type can be generated in Kotlin
https://github.com/Kotlin/kotlin-spark-api/blob/470bcf4dd6a0318a1cd0e947670f921f8f62969e/kotlin-spark-api/src/main/kotlin/org/jetbrains/kotlinx/spark/api/Encoding.kt#L186
instead of Scala.
Well, this worked, but, having a large piece of copied internal code in
your codebase is bound to cause some issues over time. And so it did...
After each major release of Spark it was a large hassle to keep
compatibility between KotlinReflection.scala and the rest of Spark.
Especially, since internal calls can change between minor releases and
break on a bytecode level. This is why we have so many releases (one for
each Scala12/13 and minor Spark combo). Plus, if Spark adds a new feature
to 3.Z, well, they can just update their ScalaReflection.scala file. We
on the other hand need to support 3.X, 3.Y, ánd 3.Z with just one codebase
(which we currently do with a preprocessor
https://github.com/raydac/java-comment-preprocessor, but it's not a
walk in the park).
Spark 3.4 was the straw that broke the camel's back. ScalaReflection.scala
changed file location
https://github.com/apache/spark/blob/branch-3.4/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/ScalaReflection.scala
and a lot was changed between the last version. It was a good wake-up call
to show that this wasn't the way forward. A simple preprocessor cannot
ensure compatibility between these versions anymore and who knows what else
will break in Spark 3.5 or with Scala 3 even.
We need a new way to encode Kotlin Data Classes while maintaining the
current flexibility but without relying on internal Spark code. Spark
version bumps (even major ones) need to be doable with minor preprocessor
changes
https://github.com/Kotlin/kotlin-spark-api/blob/470bcf4dd6a0318a1cd0e947670f921f8f62969e/kotlin-spark-api/src/main/kotlin/org/jetbrains/kotlinx/spark/api/Encoding.kt#L93.
(One version of the API for ALL spark versions is unrealistic, but one for,
say 3.0, 3.1, etc. will probably be fine)
There are probably several ways to do this:

UDT: Make data classes automatically a user-defined-type, either
with an @Annotation or using a Kotlin (2.0) compiler plugin. Downside:
UDTs (afaik) only allow user defined objects serialized inside others, such
as Tuples, not as a top-level table-like object. This could be possible,
but I don't know how. Second, I don't know how (de)serializing of other
known JVM types (such as tuples, Seqs, arrays etc) inside data classes
would work.
Java Bean: Make a compiler plugin that will convert all data classes
to something like Person1 at compile time. Top-level should now work,
but again I'm not sure about nested types.
Other clever Kotlinx reflection + Spark magic: Somehow be able to
create an Encoder for any data class using reflection without the use of
Spark internals or a compiler plugin. This would be the holy grail, but
again, no clue how to do that.
Maybe even another way. I'm no Spark expert by any means. Maybe this
gitbook
https://jaceklaskowski.gitbooks.io/mastering-spark-sql/content/spark-sql-ExpressionEncoder.html
could provide any inspiration.

Hopefully, this has given you or anyone interested enough inspiration and
explanation to give it a try :) If someone can provide a proof-of-concept,
I'd be happy to explore it further.
—
Reply to this email directly, view it on GitHub
https://github.com/Kotlin/kotlin-spark-api/issues/195#issuecomment-1726253168,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAJ4XAURX5OSHEINJ4EIVM3X3HOFLANCNFSM6AAAAAAW7OYH3U
.
You are receiving this because you were mentioned.Message ID:
@.***>

Thank you for the comprehensive answer!  I will ponder this for a bit.
I have a strong interest in using Kotlin and Spark together for a work project.  It doesn't seem like there is a hard blocker per se (I can always just use the Java API), just that some of the nice-to-haves of this library may not be available unless I contribute a fix.

@Jolanrensen we should probably take a look at the connect API: https://spark.apache.org/docs/latest/spark-connect-overview.html

@Jolanrensen we should probably take a look at the connect API: https://spark.apache.org/docs/latest/spark-connect-overview.html
Allowing the Spark driver and code to use different versions from the application code might indeed solve a lot of problems for us!
However, we'd still need a completely new way to encode data classes.

I can imagine that we'll generate pojos from used data classes. in the compile time, I know you are doing something similar in datagram, but I don't have any idea how to implement it :)
And POJOs are natively supported by Spark

@asm0dey A compiler plugin could do that :)

Hi, do you have any estimate on when (or if) 3.4+ will be supported?

@gregfriis I'm sorry, no, we currently don't have the resources to figure that out. What could help is if someone from the community could provide a proof of concept solution. That way we could better weigh the time/cost to properly support it.

Small weekend/hobby update regarding the issue:
I tried Spark Connect but locally on my machine I couldn't get it to work reliably yet. Plus it requires running Spark locally with some special script, so for now, that's not ideal.
I did experiment with the potential compiler plugin route and I do have a hunch that it might be possible :). It does require some special configuration and modules, but it should be doable.
The basic angle is: Converting annotated Kotlin data classes to something Spark sees as a Scala case class. This can automatically provide us with all supported (and even nested) types without having to traverse the classes ourselves.
In theory, this is not that difficult, but it comes with a few gotchas:

Scala case classes implement scala.Product, so we need to do that with our data classes too
Spark grabs names and type information from the primary constructor, so our primary constructor needs to have only Scala collections (only things that are supported by ScalaReflection.scala, plus, for the argument names to be readable by Scala's reflection, we need the -parameters compiler argument.
For Spark to be able to read the values, we need a function in the class without arguments and with the same name for each of the properties of the data class.

To recreate my experiment, we need:
Scala module with type retrieval
Stolen from KotlinReflection.scala, this piece of code can create a Scala Type from a Java class, something we need to make an encoder using ScalaReflection.encoderFor() later on:
object TypeRetrieval {
  val universe: scala.reflect.runtime.universe.type = scala.reflect.runtime.universe

  import universe._

  def getType[T](clazz: Class[T]): universe.Type = {
    clazz match {
      // not sure about this line, without it, no spark dependencies are needed
      case _ if clazz == classOf[Array[Byte]] => localTypeOf[Array[Byte]]
      case _ => {
        val mir = runtimeMirror(clazz.getClassLoader)
        mir.classSymbol(clazz).toType
      }
    }
  }
}

Kotlin module depending on the Scala module
This little helper function can create a Spark ExpressionEncoder for any type or class. This would be similar to calling Encoders.product[T] or ExpressionEncoder[T] from Scala.
inline fun <reified T : Any> encoderFor(): Encoder<T> = encoderFor(T::class.java)
fun <T> encoderFor(klass: Class<T>): Encoder<T> =
    ExpressionEncoder.apply(
        ScalaReflection.encoderFor(
            TypeRetrieval.getType(klass),
            false
        ) as AgnosticEncoder<T>
    )

Next, to limit the amount of generated code, we can create a little bridge from data classes to Product, implementing some stuff with reflection. This may be omitted if it seems to cause too much overhead, as it can also be generated by a compiler plugin right in the data class.
abstract class DataClassProduct(private val klass: KClass<*>) : Product {

    override fun productPrefix(): String = klass.simpleName!!

    private val params
        get() = klass
            .primaryConstructor!!
            .parameters
            .withIndex()
            .associate { (i, it) -> i to it }

    override fun canEqual(that: Any?): Boolean = that!!::class == klass
    override fun productElement(n: Int): Any = params[n]?.let { param ->
        klass.declaredMemberProperties
            .firstOrNull { it.name == param.name }
            ?.call(this)
    } ?: throw IndexOutOfBoundsException(n.toString())

    override fun productArity(): Int = params.size

    override fun productElementNames(): Iterator<String> = CollectionConverters.asScala(
        iterator {
            for (param in params.values) {
                yield(param.name!!)
            }
        }
    )
}

Writing actual Kotlin data classes
NOTE: we need the -parameters tag for compiling it, so in build.gradle.kts:
kotlin {
    jvmToolchain(8)
    compilerOptions {
        javaParameters = true
    }
}

Let's say we now want to encode a simple data class. We could just write
@SomeSparkAnnotation
data class Person(
    val name: String,
    val age: Int,
    val hobbies: List<String>,
    val address: Pair<String, Int>,
)

What the compiler plugin would produce
and then the compiler plugin could convert this to:
data class Person(
    val name: String,
    val age: Int,
    val hobbies: scala.collection.immutable.List<String>, // converting java.util.List to scala.collection.immutable.List
    val address: scala.Tuple2<String, Int>, // converting kotlin.Pair to scala.Tuple2
): DataClassProduct(Person::class) {
    // accessors for Spark
    fun name() = this.name
    fun age() = this.age
    fun hobbies() = this.hobbies
    fun address() = this.address

    companion object {
        // so we can still create it normally from kotlin
        operator fun invoke(
            name: String,
            age: Int,
            hobbies: List<String>,
            address: Pair<String, Int>,
        ): Person =
            Person(
                name = name,
                age = age,
                hobbies = CollectionConverters.asScala(hobbies).toList(),
                address = address.toTuple() // from scalaTuplesInKotlin
            )
    }
}

Running this with Spark will work correctly:
val test = Person(
    name = "Jolan",
    age = 22,
    hobbies = listOf("Programming", "Gaming"),
    address = "Kerkstraat" to 1
)
spark.createDataset(listOf(test), encoderFor<Person>()).show()
// +-----+---+--------------------+---------------+
// | name|age|             hobbies|        address|
// +-----+---+--------------------+---------------+
// |Jolan| 22|[Programming, Gam...|{Kerkstraat, 1}|
// +-----+---+--------------------+---------------+

But, we might need something more advanced, because calling test.hobbies will now result in a Scala list again :/
It does have some promise though, as nested data classes like these even work :), plus we don't need to hack into the Spark source code anymore.
How further
We could try to generate a Person_: DataClassProduct that we can auto-convert to and from by also autogenerating and registering a UDT for each data class. This UDT would then use the encoder of Person_ for all the typing information and handle the conversion from scala-kotlin types. This needs some further experimenting.
(I do have some issues calling the deserializer on an encoded row with the ExpressionEncoder, both in Scala and Kotlin, strange...)
And of course, actually build a compiler plugin. This is tricky and requires Kotlin 2.0.

Hey, this is just awesome research, thank you!
I think it makes more sense to compile to Java POJOs rather than to case classes. POJOs are natively supported by JavaReflection and should be significantly easier to generate. Moreover, it might be easy to generate syntetic (like Person_) POJOs and convert them to data classes automatically

@asm0dey you mean using Encoders.bean()?
That can indeed be done relatively easily, also generated, but this limits us in other ways again: nullability/default arguments are needed, and, for instance, nested tuples are not supported anymore:
data class AddressJava @JvmOverloads constructor(
    var street: String = "",
    var city: String = "",
) : Serializable

data class PersonJava @JvmOverloads constructor(
    var name: String = "",
    var age: Int = -1,
    var tupleTest: Tuple2<AddressJava, Int>? = null,
    var listTest: List<AddressJava> = emptyList(),
) : Serializable
...
val data = listOf(
    PersonJava("Jolan", 25, Tuple2(AddressJava("Street", "City"), 5), listOf(AddressJava("Street", "City"))),
)

val encoder = Encoders.bean(PersonJava::class.java)
val df = spark.createDataset(data, encoder)
df.show()
// +---+----------------+-----+---------+
// |age|        listTest| name|tupleTest|
// +---+----------------+-----+---------+
// | 25|[{City, Street}]|Jolan|       {}|
// +---+----------------+-----+---------+

And calling df.head() breaks everything:
Exception in thread "main" org.apache.spark.SparkRuntimeException: Error while decoding: org.apache.spark.SparkRuntimeException: Couldn't find a valid constructor on class scala.Tuple2.

This is something that is possible in the current version of the Kotlin Spark API, so it seems a bit harsh to break that.
We could extend JavaTypeInference, but we already tried to make our own
KotlinReflection.encoderFor(class) before...

Huh, right, I forgot about the tuples support. Is it possible and necessary to support default argument values? It seems that backend should not care at all because at runtime they are already known

Java bean support requires an empty constructor, so yeah :/. That's what @JvmOverloads achieves

no-args plugin already adds a noargs constructor to a data class without default arguments, see here: https://kotlinlang.org/docs/no-arg-plugin.html
You can even force it to work on your custom annotation. Doesn't solve the nested tuples feature though

2 updates:
First: We can successfully define a UDT for a class (en/de)coding it using the ExpressionEncoder of a different class. No generics are supported afaik though. So we cannot make a generic UDT for java.util.Lists which parses it using an IterableEncoder.  (Probably explains why that hasn't been done before)
Second: I tweaked the data class in such a way that it can work perfectly, both as a case class from Spark/Scala, as well as a data class from Kotlin :)
Given:
@SomeAnnotation
data class Address(
    val streetAndNumber: Pair<String, Int>,
    val city: String,
)

We generate:
class Address( // main constructor with scala names/types
    streetAndNumber: Tuple2<String, Int>,
    city: String,
) : Product {
    // secondary in companion object with kotlin names/types
    // Mainly, so Spark doesn't see this constructor
    companion object {
        operator fun invoke(
            streetAndNumber: Pair<String, Int>,
            city: String,
        ): Address = Address(
            streetAndNumber = streetAndNumber.toTuple(),
            city = city,
        )
    }

    private val scalaStreetAndNumber = streetAndNumber
    private val scalaCity = city

    // For Scala
    fun streetAndNumber() = scalaStreetAndNumber
    fun city() = scalaCity

    // For Kotlin
    val streetAndNumber = scalaStreetAndNumber.toPair()
    val city = scalaCity

    // Product functions
    override fun canEqual(that: Any?): Boolean = that is Address
    override fun productElement(n: Int): Any = when (n) {
        0 -> scalaStreetAndNumber
        1 -> scalaCity
        else -> throw IndexOutOfBoundsException(n.toString())
    }
    override fun productArity(): Int = 2

    // data class functions
    override fun equals(other: Any?): Boolean {
        if (this === other) return true
        if (other !is Address) return false

        if (streetAndNumber != other.streetAndNumber) return false
        if (city != other.city) return false

        return true
    }
    override fun hashCode(): Int {
        var result = streetAndNumber.hashCode()
        result = 31 * result + city.hashCode()
        return result
    }
    fun copy(
        streetAndNumber: Pair<String, Int> = this.streetAndNumber,
        city: String = this.city,
    ): Address = Address(
        streetAndNumber = streetAndNumber,
        city = city,
    )
    override fun toString(): String = "Address(streetAndNumber=$streetAndNumber, city='$city')"
}

Even when nested, the ExpressionEncoder is perfectly happy with this class. It can serialize and deserialize it and nested types like this and other Scala case classes (like tuples) work too.
Plus, from Kotlin we can do anything with it like it's still the original data class.
The plugin does need to create these converters:

Pair <-> Tuple2 (unless the user actually wants a Tuple2)
Triple <-> Tuple3 (same)
Java Array <-> Scala array (probably)
java.util.List <-> scala.collection.Seq
java.util.Set <-> scala.collection.Set
java.util.Map <-> scala.collection.Map
nullability <-> scala.Option (maybe?)

The rest should be good to go

Wow, man, this sounds stunning! How?

Wow, man, this sounds stunning! How?

Well if it walks like a case class and quacks like a case class XD. But jokes aside, this still requires building a Kotlin 2.0 compiler plugin to rewrite the user-written data classes on the fly, which is no easy feat, but definitely possible.

I mean the fact that you did it without released K2 is a big deal! We actually have every right to say that the future versions of Kotlin API for Apache Spark for Spark x require Kotlin 2+

Define "did it" XD. This is just a proof of concept haha. I have built no plugin yet, just an idea for what the plugin should do for it to work.

Ah, I see, misread you then. But awesome concept though!

Haha, that's okay :) But yeah, I believe something like this could do the trick.
In theory, it could even work with spark-connect (if that ever works reliably), since we don't target or modify anything specify to spark. We would still be scala-major-dependent, which is okay I think

come to think of it, we could also use the java bean serializer if, using the plugin, we could replace all scala tuples with our temporary @AllOpen data class tuples.
Depends on what is more stable :)

Yep, I think this is the way to go. Types and nullability are detected correctly and consistently now with annotations and the java bean encoder.
For unsupported data/case classes we need to generate a temporary spark-ified version, which I'll show below for Pair, but also for Tuples they would have to be generated on the fly.
Given
@Sparkify
data class Address2(
    val streetAndNumber: Pair<String, Int?>,
    var city: String,
)

we should generate
@NoArg // adding a 0-arg constructor
class Address2( // We can keep the constructor for Kotlin, the serializer doesn't use it
    streetAndNumber: Pair<String, Int?>,
    city: String,
) : Serializable {

    // Property accessors for Kotlin using a new JvmName as to not clash with the getter/setter functions
    @get:JvmName("streetAndNumber")
    @set:JvmName("streetAndNumber")
    var streetAndNumber: Pair<String, Int?> = streetAndNumber
        private set // private set if it was a val before

    @get:JvmName("city")
    @set:JvmName("city")
    var city: String = city
        public set // public set if it was a var before

    // Getters and setters for Java bean creation. The getters define the types and names of the properties.
    // These can also do conversion from/to custom types :)
    // We could probably choose to leave out these getX()/setX() functions for types that don't need to be changed
    // But for now, we'll just add them for all properties

    @Nonnull // Annotated with Nonnull for proper nullability detection
    @Deprecated("", level = DeprecationLevel.ERROR) // deprecation to prevent usage in Kotlin
    fun getCity(): String = city
    @Deprecated("", level = DeprecationLevel.ERROR)
    fun setCity(value: String) { this.city = value }

    // Since we cannot decode to kotlin.Pair, we need a custom wrapper for the Pair<String, Int>
    // Of course, annotated with @get:Nonnull/Nullable
    // This will also need to be generated each time a Triple, Tuple4, etc. is used

    @NoArg class FakePair_streetAndNumber(@get:Nonnull var first: String, @get:Nullable var second: Int?) : Serializable
    private fun Pair<String, Int?>.toFakePair_streetAndNumber() = FakePair_streetAndNumber(this.first, this.second)
    private fun FakePair_streetAndNumber.toPair() = Pair(this.first, this.second)

    @Nonnull
    @Deprecated("", level = DeprecationLevel.ERROR)
    fun getStreetAndNumber(): FakePair_streetAndNumber = this.streetAndNumber.toFakePair_streetAndNumber() // conversion
    @Deprecated("", level = DeprecationLevel.ERROR)
    fun setStreetAndNumber(value: FakePair_streetAndNumber) { this.streetAndNumber = value.toPair() }

    // Finally, just copying over the data class functions

    fun copy(
        streetAndNumber: Pair<String, Int?> = this.streetAndNumber,
        city: String = this.city,
    ): Address2 = Address2(
        streetAndNumber = streetAndNumber,
        city = city,
    )
    override fun equals(other: Any?): Boolean {
        if (this === other) return true
        if (other !is Address2) return false

        if (streetAndNumber != other.streetAndNumber) return false
        if (city != other.city) return false

        return true
    }
    override fun hashCode(): Int {
        var result = streetAndNumber.hashCode()
        result = 31 * result + city.hashCode()
        return result
    }
    override fun toString(): String = "Address(streetAndNumber=$streetAndNumber, city='$city')"
    operator fun component1(): Pair<String, Int?> = this.streetAndNumber
    operator fun component2(): String = this.city
}

With this version of the class we can still do anything with it as if it were a data class:
val addresses = listOf(
    Address2(Pair("street", 1), "city"),
    Address2(Pair("street2", null), "city2").copy(city = "city3"),
)
val (streetEtc: Pair<String, Int?>, city: String) = addresses[0]

But most importantly, we can encode and decode it using the Java bean encoder:
inline fun <reified T : Any> javaEncoderFor(): Encoder<T> = Encoders.bean(T::class.java)

javaEncoderFor<Address2>().schema().printTreeString()
// root
//  |-- city: string (nullable = false)
//  |-- streetAndNumber: struct (nullable = false)
//  |    |-- first: string (nullable = false)
//  |    |-- second: integer (nullable = true)

val ds1 = spark.createDataset(
    addresses,
    javaEncoderFor()
)
ds1.show()
// +-----+---------------+
// | city|streetAndNumber|
// +-----+---------------+
// | city|    {street, 1}|
// |city3|{street2, NULL}|
// +-----+---------------+

println(ds1.head())
// Address(streetAndNumber=(street, 1), city='city')

Do you think we should retrospectively update old versions with this implementation too?

Also, is it viable and possible to rewrite all the Scala Products to Java Beans? Or does JavaSerializer support Products OOTB?

Also, is it viable and possible to rewrite all the Scala Products to Java Beans? Or does JavaSerializer support Products OOTB?

No, products aren't supported out of the box. Otherwise, Tuples would work right away. But if a user puts a Product inside one of our data classes we could probably generate a Fake version of it in the data class, like with FakePair_streetAndNumber.
Top-level Scala products/non-sparkified data classes like Pair/Triple might still be an issue though, as we would still have to create an Encoder for those. The Java bean encoder cannot handle them... This would also prevent us from creating a Dataset from listOf(t(SomeDataClass, "something")), which might actually be a blocker :(.
If that is a blocker, I think the only other way is to create our own Kotlin version of JavaTypeInference.scala (but this time not inside the org.apache.spark.sql.catalyst package ;) ). Either in Kotlin or in Scala.
Here we would then need to add support for Kotlin data classes and
"definedByConstructorParams" classes, like Products.

IIRC the reason why I created it in that package was Spark only could read it from that package :) I would never do it without a serious reason. Also, I'm not sure if you can do it in Kotlin — probably it operates with Scala-specific reflection, but here I might be wrong.
If it's possible to implement with Kotlin and outside Spark packages - we won't be affected by changes in Spark anymore, which will bring a bearable amount of maintenance.

I'm trying the KotlinTypeInference route rn. Seems that most things work fine with Kotlin reflection since, like here, the actual encoders are somewhere else currently. This means we can add both Scala encoders and Java encoders, as well as a special product-like encoder for data classes. I'm not sure how backwards compatible that is, but it seems like the cleanest custom solution to retrieving encoders for Kotlin.
We cannot go fully custom with our encoders, as only the ones used here are compatible, but hey, we can try

It was never or goal to go fully custom, we should support the sensible subset of possibilities. Looking with a fresh eye I start to think that we actually don't need all these Scala intricacies, we need only to support the Kotlin staff and he smart about friendship delegation of everything else.
The most complex part of support - encoders for data classes and recursive generics are already written and probably should be reused.

Update time :)
I've been trying to make my own mixed version of JavaTypeInference and ScalaReflection in pure Kotlin with success!

This will allow us to create a complete replacement of the old encoding system with data classes supported out of the box :). (I did need 1 dirty hack to get the names of the columns right for data classes, but I'll get to that later haha)
One downside of this approach is that it's definitely Spark 3.4+. This is because they switched their encoding system for Spark connect to this. So, instead of defining a (de)serializer for each type, like before, they define intermediate "AgnosticEncoder" objects which later will get (de)serializers generated when building an ExpressionEncoder. This meant I had to use ProductEncoder for Kotlin data classes since they are the only Encoder that works by reading the values and instantiating a new instance of any class by calling the constructor with all arguments at once.
I'll try to make a new branch in this repo with this approach and see if we can replace the encoding with the new system. This should allow us to drop the :core module entirely and be more spark-version-independent (in theory).

And also probably significantly reduce of not remove the code preprocessing!
On Sun, 17 Mar 2024, 12:05 Jolan Rensen, @.***> wrote:

Update time :)
I've been trying to make my own mixed version of JavaTypeInference
https://github.com/apache/spark/blob/branch-3.5/sql/api/src/main/scala/org/apache/spark/sql/catalyst/JavaTypeInference.scala
and ScalaReflection
https://github.com/apache/spark/blob/branch-3.5/sql/api/src/main/scala/org/apache/spark/sql/catalyst/ScalaReflection.scala
in pure Kotlin with success!
image.png (view on web)
https://github.com/Kotlin/kotlin-spark-api/assets/17594275/bf366bf7-038c-4950-8f01-e90877e21b3b
This will allow us to create a complete replacement of the old encoding
system with data classes supported out of the box :). (I did need 1 dirty
hack to get the names of the columns right for data classes, but I'll get
to that later haha)
One downside of this approach is that it's definitely Spark 3.4+. This is
because they switched their encoding system for Spark connect to this
https://github.com/apache/spark/blob/branch-3.4/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/encoders/AgnosticEncoder.scala.
So, instead of defining a (de)serializer for each type, like before, they
define intermediate "AgnosticEncoder" objects which later will get
(de)serializers generated when building an ExpressionEncoder. This meant I
had to use ProductEncoder
https://github.com/apache/spark/blob/8c6eeb8ab0180368cc60de8b2dbae7457bee5794/sql/api/src/main/scala/org/apache/spark/sql/catalyst/encoders/AgnosticEncoder.scala#L114
for Kotlin data classes since they are the only Encoder that works by reading
the values
https://github.com/apache/spark/blob/8c6eeb8ab0180368cc60de8b2dbae7457bee5794/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/SerializerBuildHelper.scala#L350
and instantiating a new instance of any class by calling the constructor
with all arguments at once
https://github.com/apache/spark/blob/8c6eeb8ab0180368cc60de8b2dbae7457bee5794/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/DeserializerBuildHelper.scala#L353
.
I'll try to make a new branch in this repo with this approach and see if
we can replace the encoding with the new system. This should allow us to
drop the :core module entirely and be more spark-version-independent (in
theory).
—
Reply to this email directly, view it on GitHub
https://github.com/Kotlin/kotlin-spark-api/issues/195#issuecomment-2002409949,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAJ4XAV2IOLJK3MMGUHL7VDYYV2IVAVCNFSM6AAAAAAW7OYH3WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMBSGQYDSOJUHE
.
You are receiving this because you were mentioned.Message ID:
@.***>

AFAIR it should be totally possible to write our own implementation of data class instantiation. I'll try to look at it a bit later

@asm0dey I thought so too, but if we use AgnosticEncoders (which provides Spark connect compatibility as far as I know), we can only use the encoders given to us.
If we want to define our own serializers and deserializers (which should still be possible) we'll have to define them for all types again, like before.

You can check out https://github.com/Kotlin/kotlin-spark-api/pull/218 to see my progress :).
I inserted my new encoder system in the project and updated references across the board.
I disabled :core for now, but we might still need it for the vararg UDFs. I remember the Kotlin type system sorta broke down when it had to unwrap those by itself... so yeah. But it should be okay to depend on some Scala library (as long as we name it "utils" or something), as long as it doesn't contain Spark.
All other Spark helpers should be doable in pure Kotlin (as I did with debug() and debugCodegen()).
The encoders are still incomplete, so there are still many failing tests, but we can start experimenting with what does and what does not work on that branch.

One of the main problems I currently have is with the encoding of data class names using the ProductEncoder. This is because it calls the EncoderField.name() function both for the name of the column, as well as the name of the function for where to retrieve the value. readMethodName is completely ignored.
One way to tackle this is by marking you data classes like:
data class User(@get:JvmName("name") val name: String, @get:JvmName("age") val age: Int)

which sets the JVM function names of getName() and getAge() to name() and age() respectively.
However, this doesn't work for data classes we don't have access to, like Pair and Triple.
My name hack tries to override the name() function, such that only the first time it's called it returns "getAge" and the rest of the time it returns "age". This works for simple data sets (like you see in the picture), but it breaks down when the name() function is called multiple times earlier on. This for instance happens when you try to encode a List<DataClass>.
So yeah... ideas are welcome. You can turn it off by setting KotlinTypeInference.DO_NAME_HACK = false

Update: I managed to create a little compiler plugin that in the IR stage adds @JvmNames to the getters of @Sparkify annotated data classes :D. Just need to create a Gradle plugin around it so I can publish it (locally) and check if it works in combination with Spark.
Since it just touches IR it should be compatible both with K1 and K2 :).
Also, I did test that I can call the function using Java reflection. That works.
Come to think of it 🤔 I should check whether the @get:JvmName annotation is present already and skip it if so.
Plus, we might be able to add something like @ColumnName("name") (which will just be converted to @get:JvmName("name")) for custom column names :) this annotation would have to be taken into account in the encoding but that's okay.
Finally, I need to take a look at @Transient which we supported before. We could handle it with serializing from a data class, but we cannot instantiate a new data class if we don't have all constructor parameters... (Maybe @Transient requires a default argument to be present (can be warned in FIR) and @JvmOverloads does the rest? Need to test)

Yesss! it works in combination with spark

And with @ColumnName :)

Most tests are now fixed at:
https://github.com/Kotlin/kotlin-spark-api/pull/218
The PR contains the compiler+gradle plugin which can process @Sparkify and @ColumnName annotations. To build the project, both compiler-plugin and gradle-plugin need to be published to mavenLocal.
There's just one place that now fails and that is when returning a @Sparkify data class from a UDF. It's because of this Spark function which does an instance check for the value instead of using the proper deserialize method :/. So, to fix this, we need to make our data classes a scala.Product after all. I'll see what I can do in IR.

They're a hassle to make, but compiler plugins are awesome :) The compiler plugin now converts:
@Sparkify
data class User(
    val name: String,
    @ColumnName("test") val age: Int,
)

in IR successfully to:
@Sparkify
data class User(
    @get:JvmName("name") val name: String,
    @get:JvmName("test") @ColumnName("test") val age: Int,
): scala.Product {
  override fun canEqual(that: Any?): Boolean = that is User
  override fun productArity(): Int = 2
  override fun productElement(n: Int): Any? =
    if (n == 0) this.name
    else if (n == 1) this.age
    else throw IndexOutOfBoundsException()
}

Sorry to bother is there any expected release date for this

@mdsadique-inam Sorry for my absence, work called and KotlinConf and DataFrame had priority.
There are still some (large) issues with notebook support (Spark and java serialization breaks everything), so until I can solve that I won't be able to release it.
What I could do, and I plan to do that hopefully soon, is to make a pre-release of this branch https://github.com/Kotlin/kotlin-spark-api/pull/218 so you can try it for yourself and I can gather feedback :).
Unfortunately that also requires some guides/docs etc. and Kotlin DataFrame still has priority to our team, meaning my work on Spark is usually after work/in the weekends so I don't dare to put a date on it.

@mdsadique-inam Sorry for my absence, work called and KotlinConf and DataFrame had priority.
There are still some (large) issues with notebook support (Spark and java serialization breaks everything), so until I can solve that I won't be able to release it.
What I could do, and I plan to do that hopefully soon, is to make a pre-release of this branch https://github.com/Kotlin/kotlin-spark-api/pull/218 so you can try it for yourself and I can gather feedback :).
Unfortunately that also requires some guides/docs etc. and Kotlin DataFrame still has priority to our team, meaning my work on Spark is usually after work/in the weekends so I don't dare to put a date on it.

I understand your situation, so therefore I am also willing to contribute here, but don't know where to start, I am looking into it.

Also, did anyone get spark-connect to work, like at all? I'm trying to run it with a sample project with scala 2.12 (2.13 breaks) and spark 3.5.1 (or 3.5.0), on java 8, 11, any combination, but I keep getting NoSuchMethodErrors. Even though I literally follow https://spark.apache.org/docs/latest/spark-connect-overview.html#use-spark-connect-in-standalone-applications.
I want to know if I can build something spark-connect safe, but it looks like spark-connect atm is not stable enough to try :/

Alright! I finally got a working spark-connect connection using the following setup:
Spark-connect server:

Spark 3.5.1
Scala 2.12
Java 1.8

Client:

Spark-connect 3.5.1

sql api as compileOnly
spark-connect client as implementation

Scala 2.12/2.13 (both work)
JDK 17 (toolchain, gradle, project, sourceCompatibility, the whole shebang)
JVM argument "--add-opens=java.base/java.nio=ALL-UNNAMED"

I cannot add a dependency on kotlin-spark-api yet because of clashes between the normal sparkSql and this sqlApi, but let's see if I can make a "watered down" version of the API with the knowledge I have now :)

Okay, that setup breaks down when you actually try to submit classes in a jar with spark.addArtifact() because the client runs a newer java version... back to the drawing board!
Now with an actual dependence on the watered down Spark API:
Spark-connect server:

Spark 3.5.1
Scala 2.13 (as 2.12 has an intimate relationship with java 8 afaik)
Java 17 (newest possible for Spark)
spark-defaults.conf trick for netty

Spark client:

Kotlin 2.0 works
Spark-connect 3.5.1

sql api as compileOnly
spark-connect client as implementation

Scala 2.13 (library as compileOnly on all modules)
JDK (1.)8 (sourceCompatibility, targetCompatibility, jvmTarget)
Toolchain can be java 17, as long as you use "--add-opens=java.base/java.nio=ALL-UNNAMED"
use shadow and runShadow to run the project with spark.addArtifact("path to jar")

working branch: https://github.com/Kotlin/kotlin-spark-api/tree/71f115a9fa0ebec4b44e5bc3857e0fc7bacc190b

I managed to get spark-connect work in notebooks too!
https://github.com/Kotlin/kotlin-spark-api/tree/3.4%2B-spark-connect

I modified the integration with jupyter to, at the beginning of each cell, add a line that sends all created .class files to a class cache accessible by spark-connect :).
Now I need to decide how to go forward:

I either drop normal spark completely and make the kotlin-spark-api just use spark-connect. This is good for compatibility, but bad for getting started quickly and a lot of things are not supported (yet), like RDDs, UDTs (so I cannot support kotlinx.datetime etc.), UDAFs...
Or I support both spark-connect and normal spark.  Plus normal spark in notebooks I couldn't get to work at all anymore, so that's fun.

I'm happy to lend a hand, would love to see this come to fruition.

Let me know if there's a better communication channel, but happy to chat directly on how I could contribute.

@leourbina I'm always reachable at https://kotlinlang.slack.com/archives/C015B9ZRGJF for anything :) That might be a good place for ideas, discussions etc.. Better there than to flood this issue even more haha


This PR just removes the check that, in case of garbage, removes all the comment, preserving more zip data.

I'm not sure this is permitted by the ZIP spec. @davide-romanini Could you give us example(s!) of ZIP files like this?

@rylev I've added a test with a small sample of the issue.
@Plecra specs may or may be not allow for this. But reality is often different, especially for a format so old.
In my case I found that python zipfile generates garbage when opening a zip in mode a and setting a comment with a shorter length than the initial one:
>>> from zipfile import ZipFile
>>> with ZipFile('comment_garbage.zip', 'a') as z:
...     z.comment = b'long comment bla bla bla'
...
>>> with ZipFile('comment_garbage.zip', 'a') as z:
...     z.comment = b'short.'
...
>>>

This doesn't cause any problem for other zip reading libraries, so I think it should be handled even if not officially supported by the spec.

@Plecra This seems reasonable to me. Thoughts?

@davide-romanini Thanks for such a simple example 😀 This has made it very clear.
7zip lists it as a warning, but the file doesn't seem to break any of the common ZIP readers. So the question is how far we should go in supporting these files: The comment length is the only thing that bounds the time it takes to open the ZIP files (which is important for negative ID), so will we still only search within the first ~65536 bytes? I'd like to check InfoZIP's implementation for how they handle this, in case there's another oddity.

No need for this to wait any longer. Thanks for your work @davide-romanini


Xcake 0.8.8 crashes on reading Cakefile

Here is what I see in Terminal:
/Library/Ruby/Gems/2.0.0/gems/xcake-0.8.8/lib/xcake/context/xcodeproj_context.rb:43:in create_object_for_target': undefined method new_target' for nil:NilClass (NoMethodError)
from /Library/Ruby/Gems/2.0.0/gems/xcake-0.8.8/lib/xcake/context/xcodeproj_context.rb:17:in create_object_for' 	from /Library/Ruby/Gems/2.0.0/gems/xcake-0.8.8/lib/xcake/context.rb:27:in native_object_for'
from /Library/Ruby/Gems/2.0.0/gems/xcake-0.8.8/lib/xcake/generator/default_project_structure_generator.rb:27:in visit_target' 	from /Library/Ruby/Gems/2.0.0/gems/xcake-0.8.8/lib/xcake/visitor.rb:45:in visit'
from /Library/Ruby/Gems/2.0.0/gems/xcake-0.8.8/lib/xcake/dsl/project.rb:83:in block in accept' 	from /Library/Ruby/Gems/2.0.0/gems/xcake-0.8.8/lib/xcake/dsl/project.rb:82:in each'
from /Library/Ruby/Gems/2.0.0/gems/xcake-0.8.8/lib/xcake/dsl/project.rb:82:in accept' 	from /Library/Ruby/Gems/2.0.0/gems/xcake-0.8.8/lib/xcake/command/make.rb:25:in block in run'
from /Library/Ruby/Gems/2.0.0/gems/xcake-0.8.8/lib/xcake/command/make.rb:23:in each' 	from /Library/Ruby/Gems/2.0.0/gems/xcake-0.8.8/lib/xcake/command/make.rb:23:in run'
from /Library/Ruby/Gems/2.0.0/gems/claide-1.0.1/lib/claide/command.rb:334:in run' 	from /Library/Ruby/Gems/2.0.0/gems/xcake-0.8.8/bin/xcake:6:in <top (required)>'
from /usr/local/bin/xcake:23:in load' 	from /usr/local/bin/xcake:23:in '

@maximkhatskevich Could you upload an example project for me to test against ?

@jcampbell05 for some reason it does not happen anymore. After the issue appeared, I rolled back to one of the previous versions, then installed latest version again and it started to work properly. Should be something related to the environment on my computer.

@maximkhatskevich  potentially could be the xcodeproj version

maybe, because I updated everything I could while was trying to resolve the issue

Let me know if you get it again, will be good to track the version effected :)

Seeing the same problem on version 0.8.7

@frankdilo are you able to tell me what version of xcodeproj you are using ?

Hi!
I'm experiencing the same issue with Xcake 0.8.8. My version of xcodeproj is 1.4.2. Here the stacktrace, which is the same as @maximkhatskevich
  - Reading Cakefile
  - Resolving Project
  - Resolving Target<Specs>
/Users/pv/bunker/shared/bundle/ruby/2.2.0/gems/xcake-0.8.8/lib/xcake/context/xcodeproj_context.rb:43:in `create_object_for_target': undefined method `new_target' for nil:NilClass (NoMethodError)
  from /Users/pv/bunker/shared/bundle/ruby/2.2.0/gems/xcake-0.8.8/lib/xcake/context/xcodeproj_context.rb:17:in `create_object_for'
  from /Users/pv/bunker/shared/bundle/ruby/2.2.0/gems/xcake-0.8.8/lib/xcake/context.rb:27:in `native_object_for'
  from /Users/pv/bunker/shared/bundle/ruby/2.2.0/gems/xcake-0.8.8/lib/xcake/generator/default_project_structure_generator.rb:27:in `visit_target'
  from /Users/pv/bunker/shared/bundle/ruby/2.2.0/gems/xcake-0.8.8/lib/xcake/visitor.rb:45:in `visit'
  from /Users/pv/bunker/shared/bundle/ruby/2.2.0/gems/xcake-0.8.8/lib/xcake/dsl/project.rb:83:in `block in accept'
  from /Users/pv/bunker/shared/bundle/ruby/2.2.0/gems/xcake-0.8.8/lib/xcake/dsl/project.rb:82:in `each'
  from /Users/pv/bunker/shared/bundle/ruby/2.2.0/gems/xcake-0.8.8/lib/xcake/dsl/project.rb:82:in `accept'
  from /Users/pv/bunker/shared/bundle/ruby/2.2.0/gems/xcake-0.8.8/lib/xcake/command/make.rb:25:in `block in run'
  from /Users/pv/bunker/shared/bundle/ruby/2.2.0/gems/xcake-0.8.8/lib/xcake/command/make.rb:23:in `each'
  from /Users/pv/bunker/shared/bundle/ruby/2.2.0/gems/xcake-0.8.8/lib/xcake/command/make.rb:23:in `run'
  from /Users/pv/bunker/shared/bundle/ruby/2.2.0/gems/claide-1.0.1/lib/claide/command.rb:334:in `run'
  from /Users/pv/bunker/shared/bundle/ruby/2.2.0/gems/xcake-0.8.8/bin/xcake:6:in `<top (required)>'
  from /Users/pv/bunker/shared/bundle/ruby/2.2.0/bin/xcake:23:in `load'
  from /Users/pv/bunker/shared/bundle/ruby/2.2.0/bin/xcake:23:in `<main>'

Interesting seems to be due to the resolving step, I'll try to recreate this. If anyone has a project and cakefile they can upload that will be a big help.

Hi @jcampbell05
I prepared a dummy Cakefile that replicates the issue.
INSTANCE = 'Test'

project.name = 'Test'
project.organization = 'ORGANIZATION'

debug_configuration :Debug
release_configuration :Release

project.all_configurations.each do |configuration|
    configuration.settings["SWIFT_VERSION"] = "3.0"
end

target do |target|
    target.name = INSTANCE
    target.platform = :ios
    target.deployment_target = 9.3
    target.language = :swift
    target.type = :application
    target.include_files = [
        "MyApp/**/*.swift"
    ]

    target.all_configurations.each do |configuration|
        configuration.settings["ENABLE_BITCODE"] = "NO"

        configuration.settings["ASSETCATALOG_COMPILER_LAUNCHIMAGE_NAME"] = "LaunchImage"
        configuration.settings["CODE_SIGN_IDENTITY[sdk=iphoneos*]"] = "iPhone Distribution"

    end

    unit_tests_for target do |unit_test|
        unit_test.linked_targets = [target]
        unit_test.name = "Specs"
        unit_test.include_files = [
            "MyTests/**/*.*",
        ]

        unit_test.all_configurations.each do |configuration|
            configuration.settings["INFOPLIST_FILE"] = "MyTests/Info.plist"
        end
    end

end

xcake make does not always fail.  Also the above Cakefile works fine also without any other project file in the same folder :)

@jcampbell05 today I needed to regenerate project file and this issue happened again. 'xcode-select install' says I have latest tool installed. And again, the issue has been eliminated after I removed xcake at all, then installed earlier versions (I tried 0.8.7, 0.8.6, 0.8.3 - and they all were crashing). Then I removed xcake at all again and then just installed latest version available. And after that 'xcake make' worked well, no issues.

@maximkhatskevich So cleaning all Xcake versions to make sure you had 0.8.8 fixed it ?

@jcampbell05 it looks like this. But since I last time tried to fix this issue I think I only had v.0.8.8 installed anyway, so I'm not sure why it helps to switch to earlier version, then remove it and install latest version again.

Configurations defined like this:
https://github.com/jcampbell05/xcake/blob/master/example/app/CakeMania/Cakefile#L53
But even basic settings from this part (which should be applied to all Configurations that we have in the project)
https://github.com/jcampbell05/xcake/blob/master/example/app/CakeMania/Cakefile#L215
are not applied to configurations except "Debug" and "Release".

I'm not even sure when this issue was introduced, because I was only running my apps in Simulator from Xcode using "Debug" Build Configuration for the last few months. Now I tried to run my app on device using another Scheme with a custom Configuration ("RC") and realized I have big troubles with build settings.

@maximkhatskevich It's very interesting, If I can nail down where this is caused I would like to had some more integration tests and tests with the actual Cakefiles rather than just unit tests.
Seems what we have isn't enougth anymore as the tool as got more complex.

I can participate in testing, if you can give me development version and it's not too complex to install one on my computer. I have zero knowledge about how Ruby apps work and install, etc. :)

@maximkhatskevich So to test any development version of xcake all you need to do is clone the repo and just run rake install then the development version will be installed :)  I'll let you know when I have some changes.

So I've found a way of testing Xcake in a way that can reproduce this crash, simply by running xcake make, 10,000 times I am able to spot some steps change order which shouldn't be a problem as long as they don't depend on each other.
But a couple of times I have that crash but not as often as you guys seem to be getting it.
I will probably add this test into the Xcake test suite as part of the new functional tests I'm looking into adding but I am look into the underlying cause as well.

So I think I've located the problem that causes this crash, in a few cases the project is created later than expected which isn't ideal. This means that Xcake will try to create a target by calling the new_target method on nil.
This seems related to the fact some steps change order as noted above, so I'll look further into why these change.

So think I've located the issue causing this problem in #147, I simply needed to stop one of the new generators I introduced when adding support for schemes executing too early before xcake had created the project.
This caused the nil error since the project was nil. It now shouldn't execute until Xcake has finished processing the Target and Project.


fix 'fileExists' check by using stat directly

fixes #7037

any perf impact?

got an offline signoff by @mhegazy


Change item/variable name or new not popping up

When using the current ready to run windows version on Win 7 64, the popup window normally seen in blockly, or the ability to type over the label of the variables (default as "item"), does not appear to function.

Repository owner edit to trigger Github task list for this issue:

[x] Refactor Blockly to allow async update of variable names
[x] Update desktop aplication to use Ardublockly HTML modal instead of window.prompt (browser version still uses prompt)
[ ] Further refactor Blockly to allow async creation of a new variable name
[ ] Submit blockly changes as upstream PR

Looks like prompt is not supported on Electron (the framework used to created the desktop application). So it looks like an HTML implementation will have to be created to fix this.

Work around for now is renaming the variable within the "blocks xml" window.

I've got planned a simple workaround to get that working that should not take much time to implement.

Partial fix to this issue, so I'll leave it opened for now. Turned out to be a bit more complex than anticipated, as I was hoping to not have to modify core Blockly.
The functionality is accessible, but to create a new variable with a custom name an additional step is required, first you create a new variable, then rename it. I'll have to further refactor Blockly to combine both steps into 1 again.
https://github.com/carlosperate/ardublockly/commit/544cdb941949185121b82f0f98df0449ddd54f25

Set up milestone for the version after next, as partial workaround is probably adequate for first release:

[x] Refactor Blockly to allow async update of variable names
[x] Update desktop aplication to use Ardublockly HTML modal instead of window.prompt (browser version still uses prompt)
[ ] Further refactor Blockly to allow async creation of a new variable name
[ ] Submit blockly changes as upstream PR

This seems to clean things up for the time being, but it would be ideal to have the full functionality in the future.

Thanks for testing, glad the partial solution works well.


Add complete support for server-time cap

I was thinking on parsing the @time tag as a DateTime or a custom class that contained the DateTime, a Unix timestamp and a ISO string, but C# can't parse leap seconds.
Leaving it as a string leaves them the choice of parsing message timestamps to different formats.
Spec: http://ircv3.net/specs/extensions/server-time-3.2.html

Since the spec defines it as ISO 8601:2004(E) 4.3.2, we'll parse it as that. Figure it out and make it a DateTime.

Would you be opposed to using an external library that can make it easier or do you want the timestamps to be parsed as DateTime?

I still don't fully understand the problem. How are leap seconds even involved? Is the timestamp some number of seconds behind the wall clock?

Oh, I see, reading the spec. Send a patch to .NET adding support for leap seconds to DateTime and hope it gets merged before the next leap second?

I don't think failing on a leap second would be an issue. I spoke to a few IRCv3 guys in #ircdocs and I think most of the standard time libs won't handle the leap second. And reading a bit right now, I'm pretty sure leap seconds won't be sent for PCs or servers, because most of them get their time from NTP servers and when a leap second happens, NTP adjusts the time accordingly and the machines adjust their time. I'm pretty sure receiving a second as 60 is almost impossible.

Updated and also fixed the ZNC compatibility.

Thanks!


FunScript Roadmap: Compile to TypeScript and other suggestions

Hi Zach,
Two of the biggest "competitors" for FunScript have made great advances: WebSharper is now fully open source and will provide JS source maps soon, and TypeScript is getting a really capable language.
I think there's still room for FunScript for developers who don't want to adopt the whole WebSharper framework but like F# unique features like pattern matching, computation expressions and type providers. But seeing the difficulties newcomers are facing, I think it's time to get a bit more opinionated and make things easier to really catch on. Particularly:

Improve the standalone compiler, so applications don't need to compile themselves. Also, the compiler should give helpful error messages like "It seems you forgot to add the ReflectedDefinition attribute" or "FunScript doesn't know how to translate Method XXX yet, please submit a request". The compiler should also be able to remain in the background and compile on the fly when there are changes.
Now that the Community Edition of Visual Studio accepts extensions, add a command to call the compiler and a simple UI to set the options.
Integrate a JS modularity system like RequireJS to manage external dependencies.

When thinking about these and other potential improvements, I reached the conclusion that FunScript should use already existing tools as much as possible if it wants to keep up with other projects. This makes worth to consider targetting TypeScript even if it adds an extra step to compilation because:

With the existing and upcoming features, TypeScript is becoming closer and closer to F#: modules, classes, interfaces, accesors, getters and setters, lambdas which capture "this", tuples, union types, type aliases... Targetting TS would be much easier and would free FunScript from caring about Javascript oddities or the optimization and readibility of the final code.
FunScript is already using TS definitions, so a mapping between F# and TS types would likely help parsing d.ts files too, making it easier to bring back the type provider.
The TS compiler can create source maps. They won't link F#, but probably developers would find them easier to read and they can also be used by Visual Studio debugger.
TypeScript uses the same semantics for importing CommonJS (Node) or AMD (RequireJS) modules, so this is another task we could delegate to the TS compiler.

I was thinking to start working on this at the end of this month. But as you've already some ideas for the next version of FunScript I wanted to share my thoughts beforehand and avoid overlapping work as much as possible. So what do you think, Zach?
BTW, there's already another project to compile .NET code to TypeScript. It would be great if we could collaborate in some aspects.
Cheers,
Alfonso

Interesting discussion

Discussion about the new version has been moved to #171.


Oversubscription on RADV's Heap 0

Hi,
I'm running into a weird issue with VMA and RADV (default driver on Ubuntu 20.04) that I can't make sense of. I have an application that allocates 4 1920x1080 float16 images using GPU_ONLY. It seems that VMA is picking the heap 0 for this allocation and this results in a validation error:
vkAllocateMemory: attempting to allocate 16777224 bytes from heap 0,but size of that heap is only 16777216 bytes.

I can't say I understand RADV's rationale for 2 DEVICE_LOCAL heaps, but this is what it looks like on my system:
        memoryHeaps[0]:
                size   = 16777216 (0x01000000) (16.00 MiB)
                budget =<PHONE_NUMBER>3271062528 (0xffffffffe5dd3000) (16.00 EiB)
                usage  = 0 (0x00000000) (0.00 B)
                flags: count = 1
                        MEMORY_HEAP_DEVICE_LOCAL_BIT
        memoryHeaps[1]:
                size   =<PHONE_NUMBER> (0x1ff000000) (7.98 GiB)
                budget =<PHONE_NUMBER> (0x1e3dd3000) (7.56 GiB)
                usage  = 0 (0x00000000) (0.00 B)
                flags: count = 1
                        MEMORY_HEAP_DEVICE_LOCAL_BIT
        memoryHeaps[2]:
                size   =<PHONE_NUMBER> (0x1ff000000) (7.98 GiB)
                budget =<PHONE_NUMBER> (0x1faa51000) (7.92 GiB)
                usage  = 0 (0x00000000) (0.00 B)
                flags: count = 0
                        None

I'm not doing anything special with VMA just a straight forward call to vmaAllocateMemoryForImage after a call to vkCreateImage.
Any idea why heap 0 is selected as opposed to heap 1?
Thanks!

Hi, I have no idea why this driver exposes such 16 MB memory heap at the beginning. The budget of<PHONE_NUMBER>3271062528 bytes inside of it is also nonsensical, which indicates this driver is totally broken. I think you should rather open a ticket for RADV project. VMA is developed mostly on and for Windows using proprietary GPU drivers where no such situation occurs.
However, VMA is prepared to handle such situations. If allocation size exceeds the size of a memory heap, some drivers still allow to make this allocation, while some others return VK_ERROR_OUT_OF_DEVIVCE_MEMORY. In the latter case, VMA will try next eligible memory type, which may end up in heap 1. So I expect everything will work correctly and the validation layer error can just be ignored.
You can also try to make your allocations with flag VMA_ALLOCATION_CREATE_WITHIN_BUDGET_BIT. It can be used even without enabling VK_EXT_memory_budget extension. Then VMA won't try to allocate more than is available in a memory heap and so it will try another heap right away, which can solve this issue.
Please let me know if that helps.

Hi Adam,
Thanks for responding so quickly. I did as you suggested and filed an issue on Mesa's issue tracker for RADV: https://gitlab.freedesktop.org/mesa/mesa/-/issues/3845
In the interim I will try your other suggestions and report back.
Thanks.

Hi Adam,
Looks like this was an issue with RADV. There's a fix in already. I'll wait for it to show up in Ubuntu eventually.
Thanks again.


Add check to prevent overriding any of the global objects

Bad:
{% assign article = ... %}
{% assign product = ... %}
{% assign collection = ... %}

etc. All the global objects in https://shopify.dev/docs/themes/liquid/reference/objects
We should not flag for product in ....

closing as wont fix


CMUwalker can not be run

Hi, the cmuwalker example from ipythonnotebook I got error on my side:
dm_control viewer intercepted an environment error.
Original message: operands could not be broadcast together with shapes (3,) (2,)

Here is the main func:
def cmu_walker():
    walker = cmu_humanoid.CMUHumanoidPositionControlledV2020(
        observable_options={"egocentric_camera": dict(enabled=True)}
    )
    arena = corridor_arenas.WallsCorridor(
        wall_gap=3,
        wall_width=distributions.Uniform(2, 3),
        wall_height=distributions.Uniform(0.5, 1.5),
        corridor_width=4,
        corridor_length=30,
    )
    task = corridor_tasks.RunThroughCorridor(
        walker=walker,
        arena=arena,
        walker_spawn_position=(0.5, 0),
        target_velocity=3.0,
        physics_timestep=0.005,
        control_timestep=0.03,
    )
    env = composer.Environment(
        task=task,
        time_limit=10,
        random_state=np.random.RandomState(42),
        strip_singleton_obs_buffer_dim=True,
    )

    action_spec = env.action_spec()
    print(action_spec.shape)

    def random_policy(time_step):
        del time_step
        rp = np.random.uniform(
            low=action_spec.minimum, high=action_spec.maximum, size=action_spec.shape
        )
        print(rp.shape)
        return rp

    viewer.launch(env, policy=random_policy)

I don't know why the (3,) (2,) come frome, the control policy should be just (56)

shift_pose expects a position of length 3 (x, y and z), but your walker_spawn_position=(0.5, 0) only specifies x, y. Add a z-dimension and it should work.

@kevinzakka thanks! you saved my day!
may I ask another question? am pretty new to mujoco, I wanna ask, for my customized humanoid model, it's action space is 52, what's the every value means inside these 52 values？
I want using human36m GT action data to driven the model, how should I able to control the model do the exact same action like human36m? thanks aton.


std.algorithm.searching.balancedParens: make function nothrow for char types

This improves the somewhat hacky implementation of #8547 and, more generally, makes std.algorithm.searching.balancedParens nothrow when both the range's element type and E are either char, wchar, or dchar.

Not sure why the logger unittest fails on the auto-tester. My guess is it's the same problem as this: https://issues.dlang.org/show_bug.cgi?id=23286

This is diverging from the auto-decoding practice done elsewhere. Don't get me wrong, I like the change, but it makes this algorithm inconsistent with the other ones.

I see... I guess one alternative would be to wrap rn.front into a try-catch (UTFException e) block and manually use the replacement char as a fallback in the catch block (or just skip in that case?).
Did you have anything specific in mind when you wrote https://github.com/dlang/phobos/pull/8547#issuecomment-1229814392?

Did you have anything specific in mind when you wrote https://github.com/dlang/phobos/pull/8547#issuecomment-1229814392?

I was thinking of doing what you did (.byUTF on the string) at the call-site, not to change balancedParens.


bug: getrawtransaction output conflicts with zcashd output

What happened?
zcashd's getrawtransaction API is inherited from Bitcoin Core, and thus its behaviour around general transaction properties (as opposed to Zcash-specific properties) is likely ossified within the wider ecosystem (in particular for exchanges). In particular, the way it sets the confirmations field has been that way for many years.
In https://github.com/zcash/zcash/commit/f381d4e0856b5f6ebba429119f3fb97f83983c51 zcashd brought in the Insight Block Explorer patches, which added a height field alongside the confirmations field. Thus everything I say below about how height is set applies equivalently to confirmations (just with different values as appropriate).
zcashd's getrawtransaction sets height in one of three ways:

If the transaction is in the main chain, height is set to the height of the block it was mined in.
https://github.com/zcash/zcash/blob/0bcddb896c51f58771a20966a0b1f5e5110a7e52/src/main.cpp#L2085-L2088
https://github.com/zcash/zcash/blob/0bcddb896c51f58771a20966a0b1f5e5110a7e52/src/rpc/rawtransaction.cpp#L322-L329

If the transaction is in the mempool, height is omitted.
https://github.com/zcash/zcash/blob/0bcddb896c51f58771a20966a0b1f5e5110a7e52/src/main.cpp#L2064-L2069
https://github.com/zcash/zcash/blob/0bcddb896c51f58771a20966a0b1f5e5110a7e52/src/rpc/rawtransaction.cpp#L322

If the transaction is not in the mempool and not in the main chain, but is still recognized due to being mined in a block that is currently orphaned (in a non-main chain), height is set to -1.
https://github.com/zcash/zcash/blob/0bcddb896c51f58771a20966a0b1f5e5110a7e52/src/rpc/rawtransaction.cpp#L332-L335

zebrad does not implement this API, and instead sets height as follows:

If the transaction is in the main chain, height is set to the height of the block it was mined in.
https://github.com/ZcashFoundation/zebra/blob/16168d762313a9d4c2af4c30cfc3e172c3291145/zebra-rpc/src/methods.rs#L1019-L1028
https://github.com/ZcashFoundation/zebra/blob/16168d762313a9d4c2af4c30cfc3e172c3291145/zebra-rpc/src/methods.rs#L1692-L1698

If the transaction is in the mempool, height is set to -1.
https://github.com/ZcashFoundation/zebra/blob/16168d762313a9d4c2af4c30cfc3e172c3291145/zebra-rpc/src/methods.rs#L1004

If the transaction is not in the mempool and not in the main chain, but is still recognized due to being mined in a block that is currently orphaned (in a non-main chain), zebrad reports Err("transaction not found") because non-main chain blocks are not included in zebra_state::ReadResponse::Transaction.
https://github.com/ZcashFoundation/zebra/blob/16168d762313a9d4c2af4c30cfc3e172c3291145/zebra-rpc/src/methods.rs#L1029-L1031

What were you doing when the issue happened?
Debugging lightwalletd, which itself is misusing the output of getrawtransaction from zcashd: https://github.com/zcash/librustzcash/pull/1473#discussion_r1701063435
Zebra logs
No response
Zebra Version
No response
Which operating systems does the issue happen on?

[ ] Linux
[ ] macOS
[ ] Windows
[ ] Other OS

OS details
No response
Additional information
No response

This is also effectively a subset of #8646.

The fact that zebrad behaves differently than zcashd, but presumably lightwalletd clients do not adapt their behaviour to the backing node type, is further evidence to me that we cannot fix this in a backward-compatible way and shouldn't worry about doing so when changing lightwalletd.

We're going to wait until we're ready for NU6 testnet activation before prioritising this one

We also identified that the error codes returned from getrawtransaction in the case that the transaction is not found do not correspond to those from zcashd:
~/work/lightwalletd on ᚠfix/get_transaction_notfound [$] via 🐹 v1.22.2 on ☁   googleplay@z.cash(us-east1)
✗ grpcurl -d @ na-lax.zcashd.zec.rocks:443 cash.z.wallet.sdk.rpc.CompactTxStreamer/GetTransaction <<EOM
{"hash": "OVmYYbd17odbutVyi9YaHZqAbyBzixVG6q/wTdwgEFM="}
EOM
ERROR:
  Code: Unknown
  Message: -5: No such mempool or blockchain transaction. Use gettransaction for wallet transactions.

~/work/lightwalletd on ᚠfix/get_transaction_notfound [$] via 🐹 v1.22.2 on ☁   googleplay@z.cash(us-east1)
✗ grpcurl -d @ zec.rocks:443 cash.z.wallet.sdk.rpc.CompactTxStreamer/GetTransaction <<EOM
{"hash": "OVmYYbd17odbutVyi9YaHZqAbyBzixVG6q/wTdwgEFM="}
EOM
ERROR:
  Code: Unknown
  Message: 0: Transaction not found

Error codes should be drawn from https://github.com/zcash/zcash/blob/master/src/rpc/protocol.h#L32-L80

If the transaction is not in the mempool and not in the main chain, but is still recognized due to being mined in a block that is currently orphaned (in a non-main chain), height is set to -1.

@str4d, since Zebra doesn't keep orphaned blocks below the rollback limit, do you think it'd be OK to return the error "No such mempool or blockchain transaction. ..." instead of height set to -1?


add ethbridge proxy admin addresses to networks config

A couple of things:

If we change the interface, we need to add the proxyAdmin to the defaultLocalL2Network object in network.ts (I don't know what's the address though, or how to get it!).
We also need to return the address of the proxyAdmin when generating the local network, in scripts/testSetup.ts, in the getCustomNetworks() function. We might obtain it from the docker exec nitro_sequencer_1 cat /config/deployment.json command that is run when trying to get the local config.
Can you run the linter to add those missing commas and remove the warnings?


Create tradedesk-emails-update

TAMTTD has moved screenshots to local branch, made updates to existing document and submitted changes. Thank you!


Gomspace driver Enum fix

Summary

Fixes GomOuputs Enum lookup bug
Testing
Added type hinting to some gomspace driver functions. Type checker
Notes

I'll make some modifications to how the gom drivers work sometime soon as a result of this bug.
Comments

[ ] Add an x between the brackets if you commented your code well!

Just flatsat tested, works


Firefox 40 freeze

Firefox 40 seems to freeze on some websites when scrolling (yahoo.com) with the freshplayerplugin installed. Firefox 39.0.3 works fine.

Please disregard. It is apparently isolated to yahoo and who knows about them.

No, not only Yahoo. Also other sites give me the same problem. Especially in video. If FF 39.* crashes only plugin, with FF 40 to freeze and I have to kill firefox.

Confirmed here. Looks like Firefox doesn't like when I call Javascript from NPP_SetWindow. I'll look into how to do it asynchronously.

Freshplayer crashes with firefox 40. Player plugin is not load. Always the Firefox show me to reload the page but without sucess. Every page with flash has the same problem.

By the way, your freshplayer it always helped me with my works.

OK, I finally tried to postpone Javascript call, to call asynchronously, from a callback with help of NPN_PluginThreadAsyncCall, but it still hangs sometimes, waiting for some conditional variable inside NPAPI code in plugin-container.
This all most probably related to new asynchronous initialization of NPAPI plugins landed in Firefox 40. I can't say for sure yet, but looks like it wait for final initialization, which causes deadlock. Perhaps no one expected a plugin calling Javascript code from NPP_SetWindow, but it's required by freshwrapper.

Please try to disable asynchronous initialization by setting dom.ipc.plugins.asyncInit parameter to false on about:config page. Looks like it helps to fix hangs on Yahoo.com page on my machine.

I tried with that change in about: config, but the problem is not solved.
When flash crashes, freezes FF and are forced to kill it. With FF 39.* crashes only containeir and it was enough to reload the page.
I'm sorry for my bad english.

Indeed, I saw couple of times it frozen too, with asyncInit disabled. The same deadlock inside NPN_Invoke.
The bad part is when I downloaded sources and prepared to make bisection (to find changeset in Firefox code where it was started), freezes gone. :-(

I'm sorry for my bad english.

I'm not a native speaker too, so I can't tell whenever it's bad or not. As long as we understand each other, it's fine. :smile:

By the way, does https://i-rinat.github.io/flash-test-cases/ContinuousResizing/index.html hang browser?

No, it works well.
The problem is that I have not found something that I always mistake, just sometimes, mostly video and live streaming in various sites.
But when the problem by plugins, freezing the entire browser, and not just the plugin container.

freezing the entire browser

Some calls between plugin container process and browser are synchronous, so usual routine is to call and wait for answer. Bad part is that communication is two-way, therefore there is a possibility that both browser and plugin container make a call and both wait. I had similar issue in freshwrapper too for a long time; was the most annoying bug, and the most difficult to solve.

Is there a way to make issue appear less often? On my Ubuntu machine that has FF 41.0 now and the webupd8 version of the plugin, it never crashes.
On my Gentoo desktop, where i use FF 41.0 and the latest git version for the plugin, i can barely survive 10 minutes without crashing and have to reinstall FF 39.x again, which is probably really unsafe by now.
Also, is this an issue that only hits very few installations? I hardly see anyone reporting this issue anywhere.

Is there a way to make issue appear less often?

Probably, yes. Most of the time freezes I saw were caused by Javascript call from NPP_SetWindow. I've found a way to avoid that, in a6a11b58f5493415ec063fd0a05512c963218d3a. You could check whenever it helps by compiling version from dev branch.

Also, is this an issue that only hits very few installations? I hardly see anyone reporting this issue anywhere.

It only happens when timings meet certain conditions. Such bugs can happen on one machine and be absent on all others. Attaching debugger often slows program a bit, and could make bug disappear. Or appear where there were no bug. It depends.
One of the classic examples is acquiring two locks in different order. Say, first thread acquires A and then B, while second acquires B and only then A. If any of those threads manage to acquire two locks before other take a chance of capturing any, everything is fine. But if first thread captures A, and at the same time second thread captures B, they then wait indefinitely, because first thread waits for B, which is locked by second thread. And second thread waits for A, but it's locked by first thread.
Here Firefox have two processes, main and plugin container. They can make requests each other, and do them in synchronous way, with active wait for reply. Most of the time they manage to complete quickly, but if both main process and plugin container will ask each other at the same time, both block.
I need to know where plugin rectangle is located in respect to the main window. So when browser changes position, it calls NPP_SetWindow. Browser part wraps parameters of the call into a message and pass it to the plugin-container, which in turn calls NPP_SetWindow function of a plugin. My NPP_SetWindow handler evaluates some Javascript code. To get results, plugin container sends a message to the browser, which evaluates expression, and sends back a message with the results.
That increased message density meet Firefox 40 asynchronous NPAPI initialization changes, and boom! — we have a bug.
At the moment I can only workaround it by moving away from calling Javascript in NPP_SetWindow. But even if that helps, bug in inter-process communication (IPC) is still there.

Merged a6a11b58f5493415ec063fd0a05512c963218d3a into master branch. (dev was deleted).

Hello? Anybody? Does a6a11b58f5493415ec063fd0a05512c963218d3a help anyhow?

Not on my main machine. It segfaulted after 4-5 minutes.
dmesg:
Chrome_ChildThr[31692]: segfault at 0 ip<PHONE_NUMBER>4093fd sp 00007fd167ffe500 error 6 in plugin-container[400000+71000]
output:
[NPAPI 31690] WARNING: pipe error (3): Connection reset by peer: file /var/tmp/portage/www-client/firefox-41.0/work/mozilla-release/ipc/chromium/src/chrome/common/ipc_channel_posix.cc, line 459
[NPAPI 31690] ###!!! ABORT: Aborting on channel error.: file /var/tmp/portage/www-client/firefox-41.0/work/mozilla-release/ipc/glue/MessageChannel.cpp, line 1753
[NPAPI 31690] ###!!! ABORT: Aborting on channel error.: file /var/tmp/portage/www-client/firefox-41.0/work/mozilla-release/ipc/glue/MessageChannel.cpp, line 1753

"enable_windowed_mode = 0" seems a tiny bit more stable .. i "only" had 2 freezes within an hour and in both cases i could "killall plugin-container" and recover from it without Firefox fully crashing.

I was hoping that will fix it.
But anyway, on which site it freezes most often? How should I use it to make it freeze sooner? (Scrolling Yahoo title page doesn't freeze on my machine anymore.)

error 6 in plugin-container

Looks like some other error. 6 is an SIGABRT, which is raised by assertions in the code. Could you collect core file and get a backtrace from it? I'd like to know which assertion fails. Does it happen often enough? It would be nice to know where and how can I reproduce it on my machine.

Mh
With "enable_windowed_mode = 0" I also have a different result than before.
I have the same freeze, but at least crash only plugin and not FF.
However, at some sites, like streaming sports, often crashes

I was mostly using the Twitch video player and opening and closing different streams and hoping that this would trigger it faster. But the crash sometimes happens after i did anything and just opened a different tab without any flash content. So i don't have a perfect way to always trigger it.
This box just has a bit more complex setup than others. I'm using multi-head etc.
So for analysing core dumps. I enabled them will check if it at least starts creating them. However, Firefox (and 2-3 other monsters like Libreoffice) are not built with debug symbols, as this exceeds the poor tmpfs/memory resources on this machine. The plugin itself would have debug symbols though.

Firefox (and 2-3 other monsters like Libreoffice) are not built with debug symbols

Having debug information from freshplayerplugin should be enough, especially if crash is somewhere inside.

I could trigger a crash with a core dumb 2 times. However the gdb backtrace looks really worth less.
Because of:
Core was generated by `/usr/lib64/firefox/plugin-container /usr/lib64/nsbrowser/plugins/libfreshwrappe'.
I'm running:
~$ gdb /usr/lib64/firefox/plugin-container core
I still get the warning:
warning: core file may not match specified executable file.
I did not replace or rebuild any binaries between the run and the trace, so i don't know if
that warning means anything or it's just a default comment.
I cannot attach the "thread apply all bt" TXT file i did, i'm getting this every time:
"Something went really wrong, and we can’t process that file.  Try again."
Can i send the TXT (30220 bytes) somewhere?
The smaller "bt full" shows a thread that doesn't seem to have much info regarding this issue.

I had to send the .txt output by email. Github is rejecting every .txt attachment from me with:
Something went really wrong, and we can’t process that file.

I've got both files. There is no sensitive information, so I think it's fine to put bt-full.txt here:
[New LWP 12338]
[New LWP 12339]
[New LWP 12340]
[New LWP 12341]
[New LWP 12361]
[New LWP 12357]
[New LWP 12362]
[New LWP 12372]
[New LWP 12364]
[New LWP 12365]
[New LWP 12369]
[New LWP 12387]
[New LWP 12498]
[New LWP 12370]
[New LWP 12500]
[New LWP 12358]
[New LWP 12501]
[New LWP 12363]
[New LWP 12499]
[New LWP 12360]
[New LWP 12359]
[New LWP 12336]
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
Core was generated by `/usr/lib64/firefox/plugin-container /usr/lib64/nsbrowser/plugins/libfreshwrappe'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x00000000004093fd in mozalloc_abort(char const*) ()
#0  0x00000000004093fd in mozalloc_abort(char const*) ()
No symbol table info available.
#1  0x00007f29f5807620 in NS_DebugBreak () from /usr/lib64/firefox/libxul.so
No symbol table info available.
#2  0x00007f29f5ae01c0 in ?? () from /usr/lib64/firefox/libxul.so
No symbol table info available.
#3  0x00007f29f5ae0260 in ?? () from /usr/lib64/firefox/libxul.so
No symbol table info available.
#4  0x00007f29f0102c38 in event_persist_closure (ev=<optimized out>, base=0x7f29e6e97000) at /var/tmp/portage/dev-libs/libevent-2.0.22/work/libevent-2.0.22-stable/event.c:1319
        evcb_fd = 3
        evcb_callback = 0x7f29f5aabcf0
        evcb_res = <optimized out>
        evcb_arg = 0x7f29e6e9f000
#5  event_process_active_single_queue (activeq=0x7f29e6e1a680, base=0x7f29e6e97000) at /var/tmp/portage/dev-libs/libevent-2.0.22/work/libevent-2.0.22-stable/event.c:1363
        ev = <optimized out>
        count = 1
#6  event_process_active (base=<optimized out>) at /var/tmp/portage/dev-libs/libevent-2.0.22/work/libevent-2.0.22-stable/event.c:1438
        activeq = 0x7f29e6e1a680
        i = 0
        c = 0
#7  event_base_loop (base=0x7f29e6e97000, flags=1) at /var/tmp/portage/dev-libs/libevent-2.0.22/work/libevent-2.0.22-stable/event.c:1639
        evsel = 0x7f29f0335be0 <epollops>
        tv = {tv_sec =<PHONE_NUMBER>20816, tv_usec =<PHONE_NUMBER>21072}
        tv_p = <optimized out>
        res = <optimized out>
        done = 0
        retval = 0
        __func__ = "event_base_loop"
#8  0x00007f29f5aabbd8 in ?? () from /usr/lib64/firefox/libxul.so
No symbol table info available.
#9  0x00007f29f5ab19ad in ?? () from /usr/lib64/firefox/libxul.so
No symbol table info available.
#10 0x00007f29f5ac087f in ?? () from /usr/lib64/firefox/libxul.so
No symbol table info available.
#11 0x00007f29f5aba9aa in ?? () from /usr/lib64/firefox/libxul.so
No symbol table info available.
#12 0x00007f29f9c62224 in start_thread () from /lib64/libpthread.so.0
No symbol table info available.
#13 0x00007f29f3cf75ed in clone () from /lib64/libc.so.6
No symbol table info available.

As far as I can tell, debug assertion fails in receiver code in plugin-container. There are two possible explanations: (1) logic error in plugin-container code itself, or (2) freshplayerplugin code corrupts memory, and assertion fails because of that.

To give it bit more context. I've only switched between Firefox 39 and 41. I didn't update or even rebuild any other component of this machine besides Firefox and the plugin for the testing.
I've switched to freshplayerplugin in november 2014. The desktop has been running mostly 24/7 for the past 2 years and Firefox sometimes ran for weeks until i updated to version 40.

I just tested firefox 44 with last freshwrapper git and works like a charm

I'm closing this bug. Firefox is already at 46.0.1, and haven't seen any freezing for a long time now. If there would be new information, feel free to open a new issue, or reopen this one.


Refine deno.json

Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 96.29%. Comparing base (ffc67cf) to head (26a3f74).
Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #328   +/-   ##
=======================================
  Coverage   96.29%   96.29%
=======================================
  Files          36       36
  Lines        2270     2270
  Branches      224      224
=======================================
  Hits         2186     2186
  Misses         83       83
  Partials        1        1

Flag
Coverage Δ

unittests
96.29% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.


Update examples

Type

[x] Bug Fix
[ ] New Feature
[ ] Sample

Related Issues?
None
What is in this Pull Request ?
Fixed two cmdlets examples (Set-PnPBuiltInDesignPackageVisibility and Set-PnPStructuralNavigationCacheSiteState)

Thank you!


firefox 48 WebExtension compatible

change chrome.tabs.getSelected to chrome.tabs.query
change chrome.app.getDetails to chrome.runtime.getManifest
change chrome.extension.onMessage to chrome.runtime.onMessage
change chrome.extension.sendMessage to chrome.runtime.sendMessage
change chrome-extension url to data:image/png url in live.css
force popup window body width 170px

Could you resolve the conflicts ?
Thx~:D


tests, guards for #701

Add some simple tests for resource, and check we have an IPushProducer (for #701)

After some discussion with @tomprince I will just guard the resumeProducing call (i.e. instead of throwing)

We should work on https://github.com/crossbario/autobahn-python/issues/641 instead of sinking more time in dealing with HTTP2 fallouts