Skip to content

Conversation

@wujinhu
Copy link
Contributor

@wujinhu wujinhu commented Oct 18, 2022

OSSFileSystem can not work and there are some issues report it. This PR makes it work again.

#1714
#1475
#1448

@google-cla
Copy link

google-cla bot commented Oct 18, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Copy link
Member

@terrytangyuan terrytangyuan left a comment

Choose a reason for hiding this comment

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

Thank you. I don’t have a setup to test this anymore unfortunately. Could you share how you’ve tested the changes?

@wujinhu
Copy link
Contributor Author

wujinhu commented Oct 18, 2022

@terrytangyuan Thanks for your review. :) Now, all tests have passed, but there are only two tests in tests/test_ossfs.py. I think I may need add some tests or do some actual scenario tests ?

tests/test_ossfs.py::OSSFSTest::test_dir_operations PASSED
tests/test_ossfs.py::OSSFSTest::test_file_operations PASSED
tests/test_ossfs.py::OSSFSTest::test_session SKIPPED (Not a test.)

================================================================================ 2 passed, 1 skipped in 6.89s ================================================================================

@terrytangyuan
Copy link
Member

terrytangyuan commented Oct 18, 2022

Yeah do some actual tests, e.g. proving that this fixes those issues you mentioned.

@wujinhu
Copy link
Contributor Author

wujinhu commented Oct 19, 2022

@terrytangyuan yeah, I will do this work.

@wujinhu
Copy link
Contributor Author

wujinhu commented Oct 23, 2022

Tests done:

  1. existing tests(test_ossfs.py)
  2. A minimal example writes to/read from OSSFileSystem(provided in TFRecordDatasetV2 class)
  3. Add more integration testing to cover filesystem ops
collected 37 items

tests/test_ossfs.py::OSSFSTest::test_append PASSED
tests/test_ossfs.py::OSSFSTest::test_copy PASSED
tests/test_ossfs.py::OSSFSTest::test_copy_overwrite PASSED
tests/test_ossfs.py::OSSFSTest::test_copy_overwrite_false PASSED
tests/test_ossfs.py::OSSFSTest::test_create_recursive_dir PASSED
tests/test_ossfs.py::OSSFSTest::test_delete_recursively_fail PASSED
tests/test_ossfs.py::OSSFSTest::test_dir_operations PASSED
tests/test_ossfs.py::OSSFSTest::test_eof PASSED
tests/test_ossfs.py::OSSFSTest::test_file_delete PASSED
tests/test_ossfs.py::OSSFSTest::test_file_doesnt_exist PASSED
tests/test_ossfs.py::OSSFSTest::test_file_operations PASSED
tests/test_ossfs.py::OSSFSTest::test_file_read_bad_mode PASSED
tests/test_ossfs.py::OSSFSTest::test_file_write_bad_mode PASSED
tests/test_ossfs.py::OSSFSTest::test_is_directory PASSED
tests/test_ossfs.py::OSSFSTest::test_list_directory PASSED
tests/test_ossfs.py::OSSFSTest::test_list_directory_failure PASSED
tests/test_ossfs.py::OSSFSTest::test_multiple_writes PASSED
tests/test_ossfs.py::OSSFSTest::test_read PASSED
tests/test_ossfs.py::OSSFSTest::test_read_binary_mode PASSED
tests/test_ossfs.py::OSSFSTest::test_read_error_reacquires_gil PASSED
tests/test_ossfs.py::OSSFSTest::test_read_line PASSED
tests/test_ossfs.py::OSSFSTest::test_read_lines PASSED
tests/test_ossfs.py::OSSFSTest::test_reading_iterator PASSED
tests/test_ossfs.py::OSSFSTest::test_rename PASSED
tests/test_ossfs.py::OSSFSTest::test_rename_overwrite PASSED
tests/test_ossfs.py::OSSFSTest::test_rename_overwrite_false PASSED
tests/test_ossfs.py::OSSFSTest::test_seek PASSED
tests/test_ossfs.py::OSSFSTest::test_seek_from_what PASSED
tests/test_ossfs.py::OSSFSTest::test_session PASSED
tests/test_ossfs.py::OSSFSTest::test_stat PASSED
tests/test_ossfs.py::OSSFSTest::test_tell PASSED
tests/test_ossfs.py::OSSFSTest::test_utf8_string_path PASSED
tests/test_ossfs.py::OSSFSTest::test_walk_failure PASSED
tests/test_ossfs.py::OSSFSTest::test_walk_in_order PASSED
tests/test_ossfs.py::OSSFSTest::test_walk_post_order PASSED
tests/test_ossfs.py::OSSFSTest::test_write_binary_mode PASSED
tests/test_ossfs.py::OSSFSTest::test_write_to_string PASSED

===================================================================================== 37 passed in 6.09s =====================================================================================

@wujinhu
Copy link
Contributor Author

wujinhu commented Oct 24, 2022

@terrytangyuan Please take a look at this patch, thanks.

@wujinhu wujinhu requested a review from terrytangyuan October 25, 2022 07:00
Copy link
Member

@terrytangyuan terrytangyuan left a comment

Choose a reason for hiding this comment

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

Thank you!

@wujinhu
Copy link
Contributor Author

wujinhu commented Oct 31, 2022

@terrytangyuan Thank you. :)
Seems there are some build issues do not related to this change

  2022-10-30T23:21:58.3747978Z # Configuration: bcbf912f51d57f911fecea3e8ca69e5ac1654be2fb57f24d49515c0f5a6bcf52
2022-10-30T23:21:58.3748467Z # Execution platform: @local_execution_config_platform//:platform
2022-10-30T23:21:58.3749155Z cl : Command line warning D9002 : ignoring unknown option '-fvisibility=hidden'
2022-10-30T23:21:58.3749604Z cl : Command line warning D9002 : ignoring unknown option '-std=c++17'
2022-10-30T23:21:58.3750802Z C:\users\runneradmin\_bazel_runneradmin\m4q24vrm\execroot\org_tensorflow_io\bazel-out\x64_windows-fastbuild\bin\external\libapr1\_virtual_includes\libapr1\apr.h(181): fatal error C1083: Cannot open include file: 'sys/socket.h': No such file or directory
2022-10-30T23:21:58.3817290Z INFO: Elapsed time: 4454.488s, Critical Path: 59.36s
2022-10-30T23:21:58.3817940Z INFO: 5659 processes: 408 internal, 5251 local.
2022-10-30T23:21:58.7921166Z ##[error]Process completed with exit code 1.
2022-10-30T23:21:58.9201171Z Post job cleanup.
2022-10-30T23:22:01.9512439Z [command]"C:\Program Files\Git\bin\git.exe" version
+ python3.9 -m pytest --benchmark-disable -v --import-mode=append --forked --numprocesses=auto --dist loadfile ./test_documentation.py ./test_text.py ./test_parse_avro.py ./test_filter.py ./test_video.py ./test_lmdb.py ./test_genome_v1.py ./test_audio_ops.py ./test_io_tensor.py ./test_mongodb.py ./test_filesystem.py ./test_http.py ./test_feather.py ./test_json.py ./test_genome.py ./test_serial_ops.py ./test_color.py ./test_orc.py ./test_hdf5.py ./test_bigquery.py ./test_obj.py ./test_libsvm.py ./test_bigtable/test_parallel_read_rows.py ./test_bigtable/test_row_set.py ./test_bigtable/test_serialization.py ./test_bigtable/test_read_rows.py ./test_io_dataset.py ./test_pulsar.py ./test_csv.py ./test_kafka.py ./test_avro.py ./test_hdfs.py ./test_parquet.py ./test_elasticsearch.py ./test_azure.py ./test_kafka_v1.py ./test_gcs.py ./test_ffmpeg.py ./test_version.py ./test_archive.py ./test_io_layer.py ./test_dicom.py ./test_serialization.py ./test_arrow.py ./test_s3.py ./test_audio.py ./test_ossfs.py ./test_image.py ./test_pcap.py
ERROR: usage: __main__.py [options] [file_or_dir] [file_or_dir] [...]
__main__.py: error: unrecognized arguments: --forked
  inifile: None
  rootdir: /v

TFIO_DATAPATH=bazel-bin python3 -m pytest --benchmark-disable -v --import-mode=append --forked --numprocesses=auto --dist loadfile tests/test_ossfs.py

I can run in my local env

[root@iZrj9ib0wtl2kz5w23n0wiZ io]# TFIO_DATAPATH=bazel-bin python3 -m pytest --benchmark-disable -v --import-mode=append --forked --numprocesses=auto --dist loadfile tests/test_ossfs.py
==================================================================================== test session starts =====================================================================================
platform linux -- Python 3.9.6, pytest-7.1.3, pluggy-1.0.0 -- /usr/bin/python3
cachedir: .pytest_cache
benchmark: 4.0.0 (defaults: timer=time.perf_counter disable_gc=False min_rounds=5 min_time=0.000005 max_time=1.0 calibration_precision=10 warmup=False warmup_iterations=100000)
rootdir: /root/io
plugins: benchmark-4.0.0, forked-1.4.0, xdist-3.0.2
[gw0] linux Python 3.9.6 cwd: /root/io
[gw1] linux Python 3.9.6 cwd: /root/io
[gw0] Python 3.9.6 (default, Aug 25 2021, 16:22:38)  -- [GCC 8.5.0 20210514 (Red Hat 8.5.0-3)]
[gw1] Python 3.9.6 (default, Aug 25 2021, 16:22:38)  -- [GCC 8.5.0 20210514 (Red Hat 8.5.0-3)]
gw0 [37] / gw1 [37]
scheduling tests via LoadFileScheduling

tests/test_ossfs.py::OSSFSTest::test_append
[gw0] [  2%] PASSED tests/test_ossfs.py::OSSFSTest::test_append
tests/test_ossfs.py::OSSFSTest::test_copy
[gw0] [  5%] PASSED tests/test_ossfs.py::OSSFSTest::test_copy
tests/test_ossfs.py::OSSFSTest::test_copy_overwrite
[gw0] [  8%] PASSED tests/test_ossfs.py::OSSFSTest::test_copy_overwrite
tests/test_ossfs.py::OSSFSTest::test_copy_overwrite_false
[gw0] [ 10%] PASSED tests/test_ossfs.py::OSSFSTest::test_copy_overwrite_false
tests/test_ossfs.py::OSSFSTest::test_create_recursive_dir
[gw0] [ 13%] PASSED tests/test_ossfs.py::OSSFSTest::test_create_recursive_dir
tests/test_ossfs.py::OSSFSTest::test_delete_recursively_fail
[gw0] [ 16%] PASSED tests/test_ossfs.py::OSSFSTest::test_delete_recursively_fail
tests/test_ossfs.py::OSSFSTest::test_dir_operations
[gw0] [ 18%] PASSED tests/test_ossfs.py::OSSFSTest::test_dir_operations
tests/test_ossfs.py::OSSFSTest::test_eof
[gw0] [ 21%] PASSED tests/test_ossfs.py::OSSFSTest::test_eof
tests/test_ossfs.py::OSSFSTest::test_file_delete
[gw0] [ 24%] PASSED tests/test_ossfs.py::OSSFSTest::test_file_delete
tests/test_ossfs.py::OSSFSTest::test_file_doesnt_exist
[gw0] [ 27%] PASSED tests/test_ossfs.py::OSSFSTest::test_file_doesnt_exist
tests/test_ossfs.py::OSSFSTest::test_file_operations
[gw0] [ 29%] PASSED tests/test_ossfs.py::OSSFSTest::test_file_operations
tests/test_ossfs.py::OSSFSTest::test_file_read_bad_mode
[gw0] [ 32%] PASSED tests/test_ossfs.py::OSSFSTest::test_file_read_bad_mode
tests/test_ossfs.py::OSSFSTest::test_file_write_bad_mode
[gw0] [ 35%] PASSED tests/test_ossfs.py::OSSFSTest::test_file_write_bad_mode
tests/test_ossfs.py::OSSFSTest::test_is_directory
[gw0] [ 37%] PASSED tests/test_ossfs.py::OSSFSTest::test_is_directory
tests/test_ossfs.py::OSSFSTest::test_list_directory
[gw0] [ 40%] PASSED tests/test_ossfs.py::OSSFSTest::test_list_directory
tests/test_ossfs.py::OSSFSTest::test_list_directory_failure
[gw0] [ 43%] PASSED tests/test_ossfs.py::OSSFSTest::test_list_directory_failure
tests/test_ossfs.py::OSSFSTest::test_multiple_writes
[gw0] [ 45%] PASSED tests/test_ossfs.py::OSSFSTest::test_multiple_writes
tests/test_ossfs.py::OSSFSTest::test_read
[gw0] [ 48%] PASSED tests/test_ossfs.py::OSSFSTest::test_read
tests/test_ossfs.py::OSSFSTest::test_read_binary_mode
[gw0] [ 51%] PASSED tests/test_ossfs.py::OSSFSTest::test_read_binary_mode
tests/test_ossfs.py::OSSFSTest::test_read_error_reacquires_gil
[gw0] [ 54%] PASSED tests/test_ossfs.py::OSSFSTest::test_read_error_reacquires_gil
tests/test_ossfs.py::OSSFSTest::test_read_line
[gw0] [ 56%] PASSED tests/test_ossfs.py::OSSFSTest::test_read_line
tests/test_ossfs.py::OSSFSTest::test_read_lines
[gw0] [ 59%] PASSED tests/test_ossfs.py::OSSFSTest::test_read_lines
tests/test_ossfs.py::OSSFSTest::test_reading_iterator
[gw0] [ 62%] PASSED tests/test_ossfs.py::OSSFSTest::test_reading_iterator
tests/test_ossfs.py::OSSFSTest::test_rename
[gw0] [ 64%] PASSED tests/test_ossfs.py::OSSFSTest::test_rename
tests/test_ossfs.py::OSSFSTest::test_rename_overwrite
[gw0] [ 67%] PASSED tests/test_ossfs.py::OSSFSTest::test_rename_overwrite
tests/test_ossfs.py::OSSFSTest::test_rename_overwrite_false
[gw0] [ 70%] PASSED tests/test_ossfs.py::OSSFSTest::test_rename_overwrite_false
tests/test_ossfs.py::OSSFSTest::test_seek
[gw0] [ 72%] PASSED tests/test_ossfs.py::OSSFSTest::test_seek
tests/test_ossfs.py::OSSFSTest::test_seek_from_what
[gw0] [ 75%] PASSED tests/test_ossfs.py::OSSFSTest::test_seek_from_what
tests/test_ossfs.py::OSSFSTest::test_session
[gw0] [ 78%] PASSED tests/test_ossfs.py::OSSFSTest::test_session
tests/test_ossfs.py::OSSFSTest::test_stat
[gw0] [ 81%] PASSED tests/test_ossfs.py::OSSFSTest::test_stat
tests/test_ossfs.py::OSSFSTest::test_tell
[gw0] [ 83%] PASSED tests/test_ossfs.py::OSSFSTest::test_tell
tests/test_ossfs.py::OSSFSTest::test_utf8_string_path
[gw0] [ 86%] PASSED tests/test_ossfs.py::OSSFSTest::test_utf8_string_path
tests/test_ossfs.py::OSSFSTest::test_walk_failure
[gw0] [ 89%] PASSED tests/test_ossfs.py::OSSFSTest::test_walk_failure
tests/test_ossfs.py::OSSFSTest::test_walk_in_order
[gw0] [ 91%] PASSED tests/test_ossfs.py::OSSFSTest::test_walk_in_order
tests/test_ossfs.py::OSSFSTest::test_walk_post_order
[gw0] [ 94%] PASSED tests/test_ossfs.py::OSSFSTest::test_walk_post_order
tests/test_ossfs.py::OSSFSTest::test_write_binary_mode
[gw0] [ 97%] PASSED tests/test_ossfs.py::OSSFSTest::test_write_binary_mode
tests/test_ossfs.py::OSSFSTest::test_write_to_string
[gw0] [100%] PASSED tests/test_ossfs.py::OSSFSTest::test_write_to_string

==================================================================================== 37 passed in 10.29s =====================================================================================

@yongtang
Copy link
Member

yongtang commented Nov 4, 2022

@wujinhu The issue is that pytest is deprecating forked command. In the mean time, can you carry the change:

https://github.com/tensorflow/io/pull/1728/files

This will fix the test failure issue.

@wujinhu
Copy link
Contributor Author

wujinhu commented Nov 8, 2022

rebase master

@wujinhu
Copy link
Contributor Author

wujinhu commented Nov 9, 2022

@terrytangyuan @yongtang Can we merge this pr? Seems these failures do not related to this change.

@yongtang yongtang merged commit f919434 into tensorflow:master Nov 9, 2022
@yongtang
Copy link
Member

yongtang commented Nov 9, 2022

@wujinhu Thanks for the contribution!

zheolong pushed a commit to zheolong/io-1 that referenced this pull request Jul 24, 2025
* make ossfs work again

* make ossfs work again

* make ossfs work again

* fix code style
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.

3 participants