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

[C++] Skyhook still uses deprecated BufferReader(const uint8_t*, int64_t) API #37485

Closed
kou opened this issue Aug 31, 2023 · 4 comments · Fixed by #37486
Closed

[C++] Skyhook still uses deprecated BufferReader(const uint8_t*, int64_t) API #37485

kou opened this issue Aug 31, 2023 · 4 comments · Fixed by #37486
Assignees
Milestone

Comments

@kou
Copy link
Member

kou commented Aug 31, 2023

Describe the bug, including details regarding any error messages, version, and platform.

FAILED: src/skyhook/CMakeFiles/arrow_skyhook_objlib.dir/protocol/skyhook_protocol.cc.o 
/bin/ccache /bin/c++ -DARROW_EXTRA_ERROR_CONTEXT -DARROW_HAVE_RUNTIME_AVX2 -DARROW_HAVE_RUNTIME_AVX512 -DARROW_HAVE_RUNTIME_BMI2 -DARROW_HAVE_RUNTIME_SSE4_2 -DARROW_HAVE_SSE4_2 -DARROW_MIMALLOC -DARROW_WITH_RE2 -DARROW_WITH_TIMING_TESTS -DARROW_WITH_UTF8PROC -DBOOST_ALL_NO_LIB -I/home/kou/work/cpp/arrow.kou/cpp.build/src -I/home/kou/work/cpp/arrow.kou/cpp/src -I/home/kou/work/cpp/arrow.kou/cpp/src/generated -isystem /home/kou/work/cpp/arrow.kou/cpp/thirdparty/flatbuffers/include -isystem /home/kou/work/cpp/arrow.kou/cpp/thirdparty/hadoop/include -isystem /home/kou/work/cpp/arrow.kou/cpp.build/google_cloud_cpp_ep-install/include -isystem /home/kou/work/cpp/arrow.kou/cpp.build/crc32c_ep-install/include -isystem /home/kou/work/cpp/arrow.kou/cpp.build/orc_ep-install/include -isystem /home/kou/work/cpp/arrow.kou/cpp.build/awssdk_ep-install/include -isystem /home/kou/work/cpp/arrow.kou/cpp.build/xsimd_ep/src/xsimd_ep-install/include -isystem /home/kou/work/cpp/arrow.kou/cpp.build/jemalloc_ep-prefix/src -isystem /home/kou/work/cpp/arrow.kou/cpp.build/mimalloc_ep/src/mimalloc_ep/include/mimalloc-2.0 -Wno-noexcept-type -Wno-self-move  -fdiagnostics-color=always  -Wall -Wno-conversion -Wno-sign-conversion -Wunused-result -Wdate-time -fno-semantic-interposition -msse4.2  -g -Werror -O0 -ggdb -g3 -std=c++17 -fPIC -MD -MT src/skyhook/CMakeFiles/arrow_skyhook_objlib.dir/protocol/skyhook_protocol.cc.o -MF src/skyhook/CMakeFiles/arrow_skyhook_objlib.dir/protocol/skyhook_protocol.cc.o.d -o src/skyhook/CMakeFiles/arrow_skyhook_objlib.dir/protocol/skyhook_protocol.cc.o -c /home/kou/work/cpp/arrow.kou/cpp/src/skyhook/protocol/skyhook_protocol.cc
/home/kou/work/cpp/arrow.kou/cpp/src/skyhook/protocol/skyhook_protocol.cc: In function ‘arrow::Status skyhook::DeserializeScanRequest(ceph::bufferlist&, ScanRequest*)’:
/home/kou/work/cpp/arrow.kou/cpp/src/skyhook/protocol/skyhook_protocol.cc:77:88: error: ‘arrow::io::BufferReader::BufferReader(const uint8_t*, int64_t)’ is deprecated: Deprecated in 14.0.0. Use FromString or BufferReader(std::shared_ptr<Buffer> buffer) instead. [-Werror=deprecated-declarations]
   77 |                                                    request->projection_schema()->size());
      |                                                                                        ^
In file included from /home/kou/work/cpp/arrow.kou/cpp/src/arrow/io/api.h:25,
                 from /home/kou/work/cpp/arrow.kou/cpp/src/skyhook/protocol/skyhook_protocol.cc:22:
/home/kou/work/cpp/arrow.kou/cpp/src/arrow/io/memory.h:159:3: note: declared here
  159 |   BufferReader(const uint8_t* data, int64_t size);
      |   ^~~~~~~~~~~~
/home/kou/work/cpp/arrow.kou/cpp/src/skyhook/protocol/skyhook_protocol.cc:79:82: error: ‘arrow::io::BufferReader::BufferReader(const uint8_t*, int64_t)’ is deprecated: Deprecated in 14.0.0. Use FromString or BufferReader(std::shared_ptr<Buffer> buffer) instead. [-Werror=deprecated-declarations]
   79 |                                                 request->dataset_schema()->size());
      |                                                                                  ^
/home/kou/work/cpp/arrow.kou/cpp/src/arrow/io/memory.h:159:3: note: declared here
  159 |   BufferReader(const uint8_t* data, int64_t size);
      |   ^~~~~~~~~~~~
cc1plus: all warnings being treated as errors

Related change: GH-37360

Component(s)

C++

@mapleFU
Copy link
Member

mapleFU commented Aug 31, 2023

Do we have CI for skyhook? How can I avoid that I change a IO module and found the skyhook user have used the deprecated function?

@kou
Copy link
Member Author

kou commented Aug 31, 2023

We have the "test-skyhook-integration" nightly job for it.

@mapleFU
Copy link
Member

mapleFU commented Aug 31, 2023

Oh-ok, maybe we should trigger this CI if something was mark deprecated?

So sorry for not catching that

@kou
Copy link
Member Author

kou commented Aug 31, 2023

No problem. :-)

pitrou pushed a commit that referenced this issue Aug 31, 2023
### Rationale for this change

GH-37360 deprecated `BufferReader(const uint8_t*, int64_t)`.

### What changes are included in this PR?

Use `BufferReader(std::shared_ptr<Buffer>)` instead.

### Are these changes tested?

Yes.

### Are there any user-facing changes?

No.
* Closes: #37485

Authored-by: Sutou Kouhei <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
@pitrou pitrou added this to the 14.0.0 milestone Aug 31, 2023
loicalleyne pushed a commit to loicalleyne/arrow that referenced this issue Nov 13, 2023
…pache#37486)

### Rationale for this change

apacheGH-37360 deprecated `BufferReader(const uint8_t*, int64_t)`.

### What changes are included in this PR?

Use `BufferReader(std::shared_ptr<Buffer>)` instead.

### Are these changes tested?

Yes.

### Are there any user-facing changes?

No.
* Closes: apache#37485

Authored-by: Sutou Kouhei <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
dgreiss pushed a commit to dgreiss/arrow that referenced this issue Feb 19, 2024
…pache#37486)

### Rationale for this change

apacheGH-37360 deprecated `BufferReader(const uint8_t*, int64_t)`.

### What changes are included in this PR?

Use `BufferReader(std::shared_ptr<Buffer>)` instead.

### Are these changes tested?

Yes.

### Are there any user-facing changes?

No.
* Closes: apache#37485

Authored-by: Sutou Kouhei <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants