Skip to content

Conversation

bowenliang123
Copy link
Contributor

@bowenliang123 bowenliang123 commented Apr 8, 2025

Change Summary

Related issue number

Checklist

  • The pull request title is a good summary of the changes - it will be used in the changelog
  • Unit tests for the changes exist
  • Tests pass on CI
  • Documentation reflects the changes where applicable
  • My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

@github-actions github-actions bot added the relnotes-fix Used for bugfixes. label Apr 8, 2025
Copy link

codspeed-hq bot commented Apr 8, 2025

CodSpeed Performance Report

Merging #11719 will not alter performance

Comparing bowenliang123:dify-thrid-party (009a367) with main (b77ad73)

Summary

✅ 46 untouched benchmarks

Copy link
Contributor

github-actions bot commented Apr 8, 2025

Coverage report

This PR does not seem to contain any modification to coverable code.

@bowenliang123 bowenliang123 marked this pull request as ready for review April 8, 2025 14:52
@bowenliang123
Copy link
Contributor Author

Hi @sydney-runkle, would you like to help adding third-party-tests to this PR for trigging CI tests on Dify? And also review this PR if possible. Thanks.

@Viicos
Copy link
Member

Viicos commented Apr 9, 2025

Hello @bowenliang123, thanks for the PR. As per our policy for third-party tests, could you provide info about your project and how Pydantic is being used?

  • Are you relying on what you would consider unusual Pydantic behavior, or just simply defining models the intended way?
  • Have you experienced any regression on your project with a new Pydantic release?
  • Are you defining custom types, especially using __get_pydantic_core_schema__()?

Thanks

@Viicos Viicos added the awaiting author response awaiting response from issue opener label Apr 9, 2025
@bowenliang123
Copy link
Contributor Author

Hello @bowenliang123, thanks for the PR. As per our policy for third-party tests, could you provide info about your project and how Pydantic is being used?

  • Are you relying on what you would consider unusual Pydantic behavior, or just simply defining models the intended way?
  • Have you experienced any regression on your project with a new Pydantic release?
  • Are you defining custom types, especially using __get_pydantic_core_schema__()?

Thanks

Hi @Viicos , thanks for the review.

  1. Dify uses Pydantic for defining models and configs in normal ways, relying on usual behavior.
  2. Dify faces mypy static type checks violations when considering bumping Pydantic from 2.9 to 2.11, referring to chore: bump pydantic to 2.11 and pydantic-settings to 2.9 langgenius/dify#15049
  3. No, we are not defining custom types.

@Viicos Viicos added the third-party-tests Add this label on a PR to trigger 3rd party tests label Apr 17, 2025
@Viicos Viicos closed this Apr 17, 2025
@Viicos Viicos reopened this Apr 17, 2025
@Viicos
Copy link
Member

Viicos commented Apr 17, 2025

In this case, we would normally not include it in third party tests, simply because we don't expect regressions to happen if Pydantic is mostly used the intended way. The test suite doesn't seem to take long to run though, so if you can confirm me that your CI workflow won't change too much over time, I'm willing to include it.

Note that you have a bunch of warnings in tests: https://github.com/pydantic/pydantic/actions/runs/14512680436/job/40714766670?pr=11719.

Pytest should error on these so maybe you disabled it, but I would suggest taking a look at them before things start breaking.

@bowenliang123
Copy link
Contributor Author

OK, thanks for the reply. I'm closing this PR this time as you suggested.
As for the warning in Dify's CI test, most of them comes from the upstream libraries and we could do very little thing in them.
And again thanks for flourishing the Python ecosystem by introducing the third party tests in Pydantic.

@bowenliang123 bowenliang123 deleted the dify-thrid-party branch April 17, 2025 13:09
@Viicos
Copy link
Member

Viicos commented Apr 17, 2025

The test suite doesn't seem to take long to run though, so if you can confirm me that your CI workflow won't change too much over time, I'm willing to include it.

Looks like you missed this part ^?

@bowenliang123 bowenliang123 restored the dify-thrid-party branch April 18, 2025 05:51
@bowenliang123 bowenliang123 reopened this Apr 18, 2025
@bowenliang123
Copy link
Contributor Author

bowenliang123 commented Apr 18, 2025

Hi @Viicos , thanks for your reminder. I have reopened this PR, and rebased my changes on to the latest commit of the main branch. The third party tests for Dify has passed, while the failed tests are from BentoML, not part of my changes.

@Viicos
Copy link
Member

Viicos commented Apr 18, 2025

so if you can confirm me that your CI workflow won't change too much over time, I'm willing to include it.

Can you please confirm the above?

@bowenliang123
Copy link
Contributor Author

bowenliang123 commented Apr 18, 2025

Yes. The testing execution time is reaonablely short, which mainly consists of the dependency installation time with uv taking about 15s and the unit tests execution time for less than a minute.

@Viicos
Copy link
Member

Viicos commented Apr 18, 2025

Yes but I'm more asking if your CI workflow is subject to changes in the future, as this requires us to fix it whenever something changes on your end (for instance, this is what I suspect is happening with BentoML).

@bowenliang123
Copy link
Contributor Author

We have put all the unit tests (other than the integration tests relying on outter services) under the single testing script. So I would see it won't be changed often. And the unit tests have covered the usage of Pydantics in model declaration , construction and the configs.

@Viicos Viicos changed the title Adding Dify third party tests Adding Dify third-party tests Apr 18, 2025
Copy link
Member

@Viicos Viicos left a comment

Choose a reason for hiding this comment

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

@Viicos Viicos changed the title Adding Dify third-party tests Add Dify third-party tests Apr 18, 2025
@Viicos Viicos merged commit 07b33c3 into pydantic:main Apr 18, 2025
84 of 87 checks passed
@bowenliang123 bowenliang123 deleted the dify-thrid-party branch April 18, 2025 13:05
@Viicos Viicos added relnotes-ignore Omit this PR from the release notes. and removed relnotes-fix Used for bugfixes. labels Apr 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting author response awaiting response from issue opener relnotes-ignore Omit this PR from the release notes. third-party-tests Add this label on a PR to trigger 3rd party tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants