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

Feat/update json validator #6

Closed
wants to merge 2 commits into from

Conversation

sealbell
Copy link
Contributor

👀 Purpose and Importance of this PR:

  • My code conforms to the coding style of this project.
  • My code requires changes to the documentation.
  • I have updated the documentation accordingly.
  • All tests have passed.

🅰 Fixes for Issues:

Fixes 验证生成的 JSON 数据的格式和完整性,确保其符合预期的结构和内容。 #4

📝 Notes for the Reviewer:

This PR includes two main changes:

  1. Feature Update: The json_validator.py file has been updated to support flexible timestamp field names. The change_timestamp_format function can now handle any specified timestamp field name, making it more versatile.
  2. Testing: New tests have been added to test_file_processing.py to verify the functionality of the updated change_timestamp_format function with different field names.

🎯 How to Verify this PR:

  • Run the unit tests included in the tests/test_file_processing.py file to ensure that all tests pass and the new functionality works as expected.
  • Review the changes in ai_commons/file_processing/json_validator.py to understand the implementation of the flexible timestamp handling.

📑 Additional Documentation, e.g., KEPs (Telepace Enhancement Proposals), Usage Docs, etc.:

N/A

Copy link
Member

@cubxxw cubxxw left a comment

Choose a reason for hiding this comment

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

需要考虑多种 timestamp 的传入格式,

或者在这里设定一下约束:我们的数据收集层如何收集指定的约束 timestamp 字段

建议参考其他的优质开源项目如何做的。

如何对时间戳进行处理。

@cubxxw
Copy link
Member

cubxxw commented Jul 8, 2024

这个 PR 决定不合并

@cubxxw cubxxw closed this Jul 8, 2024
@kubbot
Copy link

kubbot commented Jul 8, 2024

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


This PR has been decided not to be merged

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