Skip to content

Conversation

@merlintang
Copy link

@merlintang merlintang commented May 29, 2018

…ternalRowMicroBatchWriter visible to outside of the classes

What changes were proposed in this pull request?

This PR is created to make relevant fields of KafkaStreamWriter and InternalRowMicroBatchWriter visible to outside of the classes.

How was this patch tested?

manual tests

…ternalRowMicroBatchWriter visible to outside of the classes
@merlintang
Copy link
Author

@jerryshao can you review this minor update ?

@jerryshao
Copy link
Contributor

Simply making these fields publicly accessible seems a little weird from Spark's side. Maybe we can use reflection instead.

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@merlintang
Copy link
Author

@jerryshao Actually, we can not use reflection to get this field information.

@gaborgsomogyi
Copy link
Contributor

Why is it required at all? Making things visible without proper reason is not a good idea.

@merlintang
Copy link
Author

merlintang commented Jun 26, 2018 via email

@gaborgsomogyi
Copy link
Contributor

Sounds weird. The topic names are available when the streaming app submitted. Why this can't be reused? Second option is reflection as jerryshao suggested.

@srowen srowen mentioned this pull request Jul 18, 2018
@asfgit asfgit closed this in 1a4fda8 Jul 19, 2018
@HyukjinKwon
Copy link
Member

HyukjinKwon commented Dec 13, 2018

BTW, fields in class constructors is not visible without val. For instance,

class A(fieldA: String) { }

generates:

$ javap -private A.class
Compiled from "A.scala"
public class A {
  public A(java.lang.String);
}

FYI, when val is added:

$ javap -private A.class
Compiled from "A.scala"
public class A {
  private final java.lang.String fieldA;
  public java.lang.String fieldA();
  public A(java.lang.String);
}

@HeartSaVioR
Copy link
Contributor

Yeah, that's what also I found interestingly on Scala. Constructor parameters can be used like fields in class but they're not exposed as fields, even private accessor. For case class we can pick up values in constructor parameters but it doesn't work with normal class.

zifeif2 pushed a commit to zifeif2/spark that referenced this pull request Nov 22, 2025
Closes apache#17422
Closes apache#17619
Closes apache#18034
Closes apache#18229
Closes apache#18268
Closes apache#17973
Closes apache#18125
Closes apache#18918
Closes apache#19274
Closes apache#19456
Closes apache#19510
Closes apache#19420
Closes apache#20090
Closes apache#20177
Closes apache#20304
Closes apache#20319
Closes apache#20543
Closes apache#20437
Closes apache#21261
Closes apache#21726
Closes apache#14653
Closes apache#13143
Closes apache#17894
Closes apache#19758
Closes apache#12951
Closes apache#17092
Closes apache#21240
Closes apache#16910
Closes apache#12904
Closes apache#21731
Closes apache#21095

Added:
Closes apache#19233
Closes apache#20100
Closes apache#21453
Closes apache#21455
Closes apache#18477

Added:
Closes apache#21812
Closes apache#21787

Author: hyukjinkwon <[email protected]>

Closes apache#21781 from HyukjinKwon/closing-prs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants