Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AVRO-2014 Add support for custom streams for the DataFile interfaces. #270

Merged

Conversation

mariusvniekerk
Copy link
Member

Downstream consumers will still have to implement their own relevant
Input/Outputstream subclasses.

Downstream consumers will still have to implement their own relevant
Input/Outputstream subclasses.
@mariusvniekerk
Copy link
Member Author

Rationale for this change is that we want to add avro support to apache arrow cpp and we need to easily be able to interact with non filesystem datafiles.

@mariusvniekerk
Copy link
Member Author

mariusvniekerk commented Jul 27, 2018

cc @vimota @thiru-apache

Copy link
Contributor

@vimota vimota left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, perhaps add a unit test in DataFileTests.cc that covers this new interface?

@thiru-mg
Copy link
Contributor

@mariusvniekerk Looks good to me. As @vimota mentioned, a unit test would be useful. Please assign the ticket to yourself in JIRA: https://issues.apache.org/jira/browse/AVRO-2014. Thank you

@mariusvniekerk
Copy link
Member Author

@thiru-apache don't think my user on apache jira has enough power oddly

@thiru-mg
Copy link
Contributor

thiru-mg commented Aug 1, 2018

@mariusvniekerk What is your Jira id? I'll assign the ticket to you.

@thiru-mg thiru-mg merged commit 563e79d into apache:master Oct 15, 2018
@mariusvniekerk mariusvniekerk deleted the avrocpp-datafile-constructors branch December 29, 2018 11:42
@steinio
Copy link

steinio commented Jul 12, 2019

Hi.
I am currently trying to create a DataFileWriter with an OutputStream as input argument, instead of a const char* filename, as I want to send the DataFileWriter encoded data over a Kafka buffer.

There is no examples on how to do this, so my attempt was to do this:

c::scheme scheme;
scheme.x = 1;
scheme.y = -1

std::unique_ptr<avro::OutputStream> out = avro::memoryOutputStream();
avro::DataFileWriter<c::scheme> dfw(std::move(out), scheme);

which compiles, but when I run it I get an error due to
filename_(NULL),
in the DataFileWriterBase constructor.
Have I done anything wrong when setting up the DataFileWriter, if so, how do I go about setting this up?
If not, what can this issue come from?

I noticed that you were going to make a test for this in DataFileTests.cc, but can't find any test for this OutputStream support in DataFileWriter.

@ghost
Copy link

ghost commented Dec 13, 2019

Hi - we are working to upgrade our codebase from 1.8 to 1.9, and we noticed that we also modified DataFileWriter to accept an OutputStream. However, our API takes the OutputStream by reference, instead of unique_ptr. Apart from being more consistent with, e.g., Encoder::init(OutputStream& os), this is also a bit more useful. See the comment above: how do you get back the stream that was written to out if out is moved into dfw?

Can I suggest to at least add an alternative API to take the stream by ref, and possibly deprecate this one? I am willing to open a PR.

@thiru-mg
Copy link
Contributor

Hi, thanks for your interest.

Please go ahead. The current implementation uses a unique_ptr and hence owns the OutputStream. If a reference is passed, this ownership model will change. We should somehow indicate the the OutputStreamBase class that it does not own the underlying ostream.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants