[ 
https://issues.apache.org/jira/browse/BEAM-6820?focusedWorklogId=217926&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-217926
 ]

ASF GitHub Bot logged work on BEAM-6820:
----------------------------------------

                Author: ASF GitHub Bot
            Created on: 25/Mar/19 10:40
            Start Date: 25/Mar/19 10:40
    Worklog Time Spent: 10m 
      Work Description: goldfishy commented on issue #8069: [BEAM-6820] 
Cassandra object mapper abstraction
URL: https://github.com/apache/beam/pull/8069#issuecomment-476140757
 
 
   
   
   > Thanks a lot, a really nice PR/idea, extending out of Cassandra object 
mapping is definitely a valuable improvement.
   
   Thank you for reviewing, very valid and insightful comments!
   
   > After doing the review I ask myself the question, do we really need 
`MapperFactory`, maybe just putting `withMapper` will be enough. We already 
have the default object mapping and if users want to provide theirs they can do 
it as they wish. Please consider removing it.
   > 
   Yes and no. @stankiewicz and myself have thought extensive about this, here 
are the options we see:
   
   **1: Remove `MapperFactory` and pass the instantiated mapper directly into 
the `CassandraIO` with `withMapper()`**
   
   - Pro: Easy interface
   - Pro: Easy to understand
   - Con: The mapper would be instantiated in the `main()` and as such would 
not have access to Cassandra session. User would have to create their own 
session. This could be hacked around for the default implementation but I 
suspect most users would want to have access to the `CassandraIO` session 
anyway. 
   
   **2: Use current suggested implementation with `MapperFactory` interface and 
`Mapper` interface:**
   
   - Pro: User gets flexibility to inject any variables they want through the 
`MapperFactory` as its instantiated in the main. 
   - Pro: Easy to understand.
   - Con: `getMapper(Session session, Class<T> entity);` is a fairly ugly 
signature.
   
   **3: Remove MapperFactory and make Mapper abstract with a forced constructor 
that takes session and entity as arguments. Make withMapper take a Class 
referring to the Mapper. Instantiate Mapper inside CassandraIO with 
reflection.** 
   
   - Pro: Removes `MapperFactory`
   - Pro: User only has to implement mapping functions.
   - Con: As the Mapper would be instantiated with reflection inside 
`CassandraIO` the user would not be able to pass any arguments to the mapper 
and would only have access to `Session` and `Entity`. 
   
   
   > Can you also rebase it (we rollback a PR of Cassandra that produced a 
conflict in the tests).
   > Then please move the classes from the mapper package to the main cassandra 
package and make the Default implementation(s) package private.
   > 
   Done!
   
   > If you use IntelliJ please do a pass of `Analyze→ Inspect Code` and fix 
any issue (if so) on the new code.
   
   Yep, will do once we decide on major discussion above. 
   
 
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Issue Time Tracking
-------------------

    Worklog Id:     (was: 217926)
    Time Spent: 4h  (was: 3h 50m)

> Custom Row-Object mapper implementation for CassandraIO
> -------------------------------------------------------
>
>                 Key: BEAM-6820
>                 URL: https://issues.apache.org/jira/browse/BEAM-6820
>             Project: Beam
>          Issue Type: Improvement
>          Components: io-java-cassandra
>            Reporter: Max Charas
>            Assignee: Max Charas
>            Priority: Minor
>          Time Spent: 4h
>  Remaining Estimate: 0h
>
> The current Cassandra source sink is tightly coupled to the Datastax Object 
> Mapper. [This requires 
> users|https://docs.datastax.com/en/developer/java-driver/3.1/manual/object_mapper/]
>  of the sink to provide a POJO describing  the table in Cassandra. Although 
> the POJO is a easy and powerful way to describe the table it does requires 
> users to always recompile the pipeline for each change in the table 
> definition. 
> I suggest adding a abstraction layer that allows users to inject their own 
> mapper implementation. One example use would be a mapper that works with a 
> generic 
> [Row|https://beam.apache.org/releases/javadoc/2.4.0/org/apache/beam/sdk/values/Row.html]
>  implementation rather than a compile-time POJO. 
> This abstraction layer could be implemented by simply supplying a 
> MapperFactory through the Sink/Source builder. 



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to