I took a look and added comments.   I'm not sure if Bob replied off-list,
so hopefully no work was duplicated.

Lets try to be mindful that the project is asynchronous in nature and it
might take a little time to reply.

Cheers,
Micah

On Thu, Apr 15, 2021 at 10:00 AM Nate Bauernfeind <
natebauernfe...@deephaven.io> wrote:

> > I think checking in the java files is fine and probably better then
> relying
> > on a third party package.  We should make sure there are instructions on
> > how to regenerate them along with the PR
>
> Micah,
>
> I just opened a pull-request to satisfy ARROW-12111. This is my first
> contribution to an apache project; please let me know if there is anything
> else that I need to do to get this past the finish line.
>
> https://github.com/apache/arrow/pull/10058
>
> Thanks,
> Nate
>
> On Wed, Apr 14, 2021 at 11:45 PM Nate Bauernfeind <
> natebauernfe...@deephaven.io> wrote:
>
> > Hey Bob,
> >
> > Someone did publish a 1.12 version of the flatc maven plugin. I double
> > checked that the plugin and binaries look correct and legit.. but you
> know,
> > it's always shady to download some random executable from the internet
> and
> > run it. However, I have been using it to generate the arrow flatbuffer
> > files because I _really_ wanted some features that are in flatc 1.12's
> > runtime jar (there are performance improvements for array types in
> > particular).
> >
> > You can see them here:
> > https://search.maven.org/search?q=com.github.shinanca
> > The repository fork is here: https://github.com/shinanca/flatc
> >
> > On the bright side that developer appears to have published an x86_64
> > windows binary which might satisfy one of your earlier complaints in the
> > thread.
> >
> > On the other hand, if everyone is comfortable checking in the flatc
> > generated files (obviously with the additional documentation on how to
> > re-generate should the fbs files change), it's a relatively small change
> to
> > replace the existing apache/arrow/java/format source. Based on the
> previous
> > discussion on this thread, it seems that the arrow dev team _is_
> > comfortable with the check-in-the-generated-files approach.
> >
> > Although 4.0 is near the release phase, there are still a few blocking
> > issues that people are trying to fix (according to the arrow-sync call
> > earlier today). I don't mind jumping in and doing this; it appears that
> > there might be enough time for such a small change to make it into the
> > release if the work is performed and merged ASAP.
> >
> > I guess, I'm either looking for the "pull request is on the way" or the
> > "thumbs up - we definitely want this; I'll get the code review for you
> when
> > it's ready" style reply =D.
> >
> > On Wed, Apr 14, 2021 at 10:43 PM Bob Tinsman <bobti...@gmail.com> wrote:
> >
> >> I apologize for leaving this hanging, but it looks like 4.0 is leaving
> >> the station :(
> >> Yes, it makes sense to bump it to 1.12, but you can't do that in
> >> isolation, because the flatc binary which is fetched as a Maven
> dependency
> >> is only available for 1.9. I will get back onto this and finish it this
> >> week.
> >>
> >> FWIW, I was looking around and catalogued the various ways of generating
> >> flatbuffers for all the languages--you can look at it in my branch:
> >> https://github.com/bobtins/arrow/tree/check-in-gen-code/java/dev
> >> Let me know if any info is wrong or missing.
> >> The methods of generation are all over the map, and some have no script
> >> or build file, just doc. Would there be any value in making this more
> >> uniform?
> >>
> >> On 2021/04/14 16:36:47, Nate Bauernfeind <natebauernfe...@deephaven.io>
> >> wrote:
> >> > It would also be nice to upgrade that java flatbuffer version from 1.9
> >> to
> >> > 1.12. Is anyone planning on doing this work (as listed in
> ARROW-12111)?
> >> >
> >> > If I did this work today, might it be possible to get it included in
> the
> >> > 4.0.0 release?
> >> >
> >> > On Fri, Mar 26, 2021 at 3:25 PM bobtins <bobti...@gmail.com> wrote:
> >> >
> >> > > OK, originally this was part of
> >> > > https://issues.apache.org/jira/browse/ARROW-12006 and I was going
> to
> >> just
> >> > > add some doc on flatc, but I will make this a new bug because it's a
> >> little
> >> > > bigger: https://issues.apache.org/jira/browse/ARROW-12111
> >> > >
> >> > > On 2021/03/23 23:40:50, Micah Kornfield <emkornfi...@gmail.com>
> >> wrote:
> >> > > > >
> >> > > > > I have a concern, though. Four other languages (Java would be
> >> five)
> >> > > check
> >> > > > > in the generated flatbuffers code, and it appears (based on a
> >> quick
> >> > > scan of
> >> > > > > Git logs) that this is done manually. Is there a danger that the
> >> binary
> >> > > > > format could change, but some language might get forgotten, and
> >> thus be
> >> > > > > working with the old format?
> >> > > >
> >> > > > The format changes relatively slowly and any changes at this point
> >> should
> >> > > > be backwards compatible.
> >> > > >
> >> > > >
> >> > > >
> >> > > > > Or is there enough interop testing that the problem would get
> >> caught
> >> > > right
> >> > > > > away?
> >> > > >
> >> > > > In most cases I would expect integration tests to catch these
> types
> >> of
> >> > > > error.
> >> > > >
> >> > > > On Tue, Mar 23, 2021 at 4:26 PM bobtins <bobti...@gmail.com>
> wrote:
> >> > > >
> >> > > > > I'm happy to check in the generated Java source. I would also
> >> update
> >> > > the
> >> > > > > Java build info to reflect this change and document how to
> >> regenerate
> >> > > the
> >> > > > > source as needed.
> >> > > > >
> >> > > > > I have a concern, though. Four other languages (Java would be
> >> five)
> >> > > check
> >> > > > > in the generated flatbuffers code, and it appears (based on a
> >> quick
> >> > > scan of
> >> > > > > Git logs) that this is done manually. Is there a danger that the
> >> binary
> >> > > > > format could change, but some language might get forgotten, and
> >> thus be
> >> > > > > working with the old format? Or is there enough interop testing
> >> that
> >> > > the
> >> > > > > problem would get caught right away?
> >> > > > >
> >> > > > > I'm new to the project and don't know how big of an issue this
> is
> >> in
> >> > > > > practice. Thanks for any enlightenment.
> >> > > > >
> >> > > > > On 2021/03/23 07:39:16, Micah Kornfield <emkornfi...@gmail.com>
> >> wrote:
> >> > > > > > I think checking in the java files is fine and probably better
> >> then
> >> > > > > relying
> >> > > > > > on a third party package.  We should make sure there are
> >> > > instructions on
> >> > > > > > how to regenerate them along with the PR
> >> > > > > >
> >> > > > > > On Monday, March 22, 2021, Antoine Pitrou <anto...@python.org
> >
> >> > > wrote:
> >> > > > > >
> >> > > > > > >
> >> > > > > > > Le 22/03/2021 à 20:17, bobtins a écrit :
> >> > > > > > >
> >> > > > > > >> TL;DR: The Java implementation doesn't have generated
> >> flatbuffers
> >> > > code
> >> > > > > > >> under source control, and the code generation depends on an
> >> > > > > > >> unofficially-maintained Maven artifact. Other language
> >> > > > > implementations do
> >> > > > > > >> check in the generated code; would it make sense for this
> to
> >> be
> >> > > done
> >> > > > > for
> >> > > > > > >> Java as well?
> >> > > > > > >>
> >> > > > > > >> I'm currently focusing on Java development; I started
> >> building on
> >> > > > > Windows
> >> > > > > > >> and got a failure under java/format, because I couldn't
> >> download
> >> > > the
> >> > > > > > >> flatbuffers compiler (flatc) to generate Java source.
> >> > > > > > >> The artifact for the flatc binary is provided
> "unofficially"
> >> (not
> >> > > by
> >> > > > > the
> >> > > > > > >> flatbuffers project), and there was no Windows version, so
> I
> >> had
> >> > > to
> >> > > > > jump
> >> > > > > > >> through hoops to build it and proceed.
> >> > > > > > >>
> >> > > > > > >
> >> > > > > > > While this does not answer the more general question of
> >> checking
> >> > > in the
> >> > > > > > > generated Flatbuffers code (which sounds like a good idea,
> >> but I'm
> >> > > not
> >> > > > > a
> >> > > > > > > Java developer), note that you could workaround this by
> >> installing
> >> > > the
> >> > > > > > > Conda-provided flatbuffers package:
> >> > > > > > >
> >> > > > > > >   $ conda install flatbuffers
> >> > > > > > >
> >> > > > > > > which should get you the `flatc` compiler, even on Windows.
> >> > > > > > > (see https://docs.conda.io/projects/conda/en/latest/ for
> >> > > installing
> >> > > > > conda)
> >> > > > > > >
> >> > > > > > > You may also try other package managers such as Chocolatey:
> >> > > > > > >
> >> > > > > > >   https://chocolatey.org/packages/flatc
> >> > > > > > >
> >> > > > > > > Regards
> >> > > > > > >
> >> > > > > > > Antoine.
> >> > > > > > >
> >> > > > > >
> >> > > > >
> >> > > >
> >> > >
> >> >
> >> >
> >> > --
> >> >
> >>
> >
> >
> > --
> >
> >
>
> --
>

Reply via email to