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

chore: bump-pydantic to 2.3.0 #375

Merged
merged 18 commits into from
Sep 8, 2023
Merged

chore: bump-pydantic to 2.3.0 #375

merged 18 commits into from
Sep 8, 2023

Conversation

@ThibaultFy ThibaultFy force-pushed the chore/bump-to-pydantic-2 branch from 429fc44 to ac833b6 Compare August 30, 2023 09:24
@ThibaultFy ThibaultFy changed the title chore: bump-pydantic tool chore: bump-pydantic to 2.3.0 Aug 31, 2023
@ThibaultFy
Copy link
Member Author

/e2e

@Owlfred

This comment was marked as outdated.

@ThibaultFy
Copy link
Member Author

/e2e --refs substrafl=chore/bump-to-pydantic-2

@Owlfred
Copy link

Owlfred commented Aug 31, 2023

End to end tests: ❌ FAILURE

Jobs status:

  • Camelyon: ⏭️
  • Dispatch Jobs: ✔️
  • Distributed / distributed-sdk:
  • MNIST: ⏭️
  • Standalone / standalone-sdk:

Sorry, try again.

@ThibaultFy
Copy link
Member Author

/e2e --refs substrafl=chore/bump-to-pydantic-2 substra=chore/bump-to-pydantic-2 substra-tests=chore/bump-to-pydantic-2 --tests sdk,substrafl --benchmark mnist,camelyon

@Owlfred
Copy link

Owlfred commented Aug 31, 2023

Something happened while processing your message: error: unknown option '--benchmark'
(Did you mean --benchmarks?)

@ThibaultFy
Copy link
Member Author

/e2e --refs substrafl=chore/bump-to-pydantic-2 substra=chore/bump-to-pydantic-2 substra-tests=chore/bump-to-pydantic-2 --tests sdk,substrafl --benchmarks mnist,camelyon

@Owlfred
Copy link

Owlfred commented Aug 31, 2023

End to end tests: ❌ FAILURE

Jobs status:

  • Camelyon / standalone-camelyon:
  • Dispatch Jobs: ✔️
  • Distributed / distributed-sdk,substrafl:
  • MNIST / standalone-mnist:
  • Standalone / standalone-sdk,substrafl:

“You shall not pass!” ― Gandalf, The Lord of the Rings, The Fellowship of the Ring

@ThibaultFy
Copy link
Member Author

/e2e --refs substrafl=chore/bump-to-pydantic-2 substra=chore/bump-to-pydantic-2 substra-tests=chore/bump-to-pydantic-2 --tests sdk,substrafl --benchmarks mnist,camelyon

@Owlfred
Copy link

Owlfred commented Aug 31, 2023

End to end tests: ❌ FAILURE

Jobs status:

  • Camelyon / standalone-camelyon:
  • Dispatch Jobs: ✔️
  • Distributed / distributed-sdk,substrafl:
  • MNIST / standalone-mnist:
  • Standalone / standalone-sdk,substrafl:

“Success is not final; failure is not fatal: It is the courage to continue that counts.” ―- Winston S. Churchill

@ThibaultFy
Copy link
Member Author

/e2e --refs substrafl=chore/bump-to-pydantic-2,substra=chore/bump-to-pydantic-2,substra-tests=chore/bump-to-pydantic-2 --tests sdk,substrafl --benchmarks mnist,camelyon

@Owlfred
Copy link

Owlfred commented Aug 31, 2023

End to end tests: ❌ FAILURE

Jobs status:

  • Camelyon / standalone-camelyon:
  • Dispatch Jobs: ✔️
  • Distributed / distributed-sdk,substrafl:
  • MNIST / standalone-mnist:
  • Standalone / standalone-sdk,substrafl:

Too bad.

@ThibaultFy
Copy link
Member Author

/e2e --refs substrafl=chore/bump-to-pydantic-2,substra=chore/bump-to-pydantic-2,substra-tests=chore/bump-to-pydantic-2,substra-backend=chore/bump-to-pydantic-2 --tests sdk,substrafl --benchmarks mnist,camelyon

@Owlfred
Copy link

Owlfred commented Aug 31, 2023

End to end tests: ❌ FAILURE

Jobs status:

  • Camelyon / standalone-camelyon:
  • Dispatch Jobs: ✔️
  • Distributed / distributed-sdk,substrafl:
  • MNIST / standalone-mnist:
  • Standalone / standalone-sdk,substrafl:

Too bad.

@ThibaultFy
Copy link
Member Author

/e2e --refs substrafl=chore/bump-to-pydantic-2,substra=chore/bump-to-pydantic-2,substra-tests=chore/bump-to-pydantic-2,substra-backend=chore/bump-to-pydantic-2 --tests sdk,substrafl --benchmarks mnist,camelyon

@Owlfred
Copy link

Owlfred commented Sep 1, 2023

End to end tests: ❌ FAILURE

“You shall not pass!” ― Gandalf, The Lord of the Rings, The Fellowship of the Ring

@ThibaultFy
Copy link
Member Author

/e2e --refs substrafl=chore/bump-to-pydantic-2,substra=chore/bump-to-pydantic-2,substra-tests=chore/bump-to-pydantic-2,substra-backend=chore/bump-to-pydantic-2 --tests sdk,substrafl --benchmarks mnist,camelyon

@Owlfred
Copy link

Owlfred commented Sep 1, 2023

End to end tests: ❌ FAILURE

Jobs status:

  • Camelyon / standalone-camelyon:
  • Dispatch Jobs: ✔️
  • Distributed / distributed-sdk,substrafl:
  • MNIST / standalone-mnist:
  • Standalone / standalone-sdk,substrafl:

“It’s just a flesh wound.” ― The Black Knight, Monty Python and the Holy Grail

@ThibaultFy
Copy link
Member Author

/e2e --refs substrafl=chore/bump-to-pydantic-2,substra=chore/bump-to-pydantic-2,substra-tests=chore/bump-to-pydantic-2,substra-backend=chore/bump-to-pydantic-2 --tests sdk,substrafl --benchmarks mnist,camelyon --mode standalone

@Owlfred
Copy link

Owlfred commented Sep 6, 2023

End to end tests: ❌ FAILURE

Jobs status:

  • Camelyon / standalone-camelyon:
  • Dispatch Jobs: ✔️
  • Distributed: ⏭️
  • MNIST / standalone-mnist:
  • Standalone / standalone-sdk,substrafl:

Sorry, try again.

@linear
Copy link

linear bot commented Sep 6, 2023

FL-1068 Support pydantic v2.0

Context

Following the recent release of pydantic v2.0 and related changelog, the code need to be adapted. In particular, some methods have been deprecated like validator and root_validator, resulting in multiple CI failures.

More details here.

Specification

  • Usages of pydantic in substra SDK need to be adapted so they comply with v2.0
  • Usages of pydantic in substrafl library need to be adapted so they comply with v2.0

Acceptance criteria

Tests passing on substra, substrafl with pydantic v2.0

Documentation examples relying on both substra and substrafl running with pydantic v2.0

CI passing on e2e distributed, standalone, benchmark tests and documentation examples with pydantic v2.0

@ThibaultFy
Copy link
Member Author

/e2e --refs substrafl=chore/bump-to-pydantic-2,substra=chore/bump-to-pydantic-2,substra-tests=chore/bump-to-pydantic-2 --tests sdk,substrafl --benchmarks mnist,camelyon --mode standalone

@Owlfred
Copy link

Owlfred commented Sep 6, 2023

End to end tests: ❌ FAILURE

Jobs status:

  • Camelyon / standalone-camelyon:
  • Dispatch Jobs: ✔️
  • Distributed: ⏭️
  • MNIST / standalone-mnist:
  • Standalone / standalone-sdk,substrafl:

Oh well.

@ThibaultFy
Copy link
Member Author

/e2e --refs substrafl=chore/bump-to-pydantic-2,substra=chore/bump-to-pydantic-2,substra-tests=chore/bump-to-pydantic-2,substra-backend=chore/bump-to-pydantic-2 --tests sdk,substrafl --benchmarks mnist,camelyon --mode standalone

@Owlfred
Copy link

Owlfred commented Sep 6, 2023

End to end tests: ❌ FAILURE

Jobs status:

  • Camelyon / standalone-camelyon:
  • Dispatch Jobs: ✔️
  • Distributed: ⏭️
  • MNIST / standalone-mnist:
  • Standalone / standalone-sdk,substrafl:

Not this time.

@ThibaultFy
Copy link
Member Author

/e2e --refs substrafl=chore/bump-to-pydantic-2,substra=chore/bump-to-pydantic-2,substra-tests=chore/bump-to-pydantic-2,substra-backend=chore/bump-to-pydantic-2 --tests sdk,substrafl --benchmarks mnist,camelyon --mode standalone

Signed-off-by: ThibaultFy <50656860+ThibaultFy@users.noreply.github.com>
Signed-off-by: ThibaultFy <50656860+ThibaultFy@users.noreply.github.com>
Signed-off-by: ThibaultFy <50656860+ThibaultFy@users.noreply.github.com>
Signed-off-by: ThibaultFy <50656860+ThibaultFy@users.noreply.github.com>
Signed-off-by: ThibaultFy <50656860+ThibaultFy@users.noreply.github.com>
Signed-off-by: ThibaultFy <50656860+ThibaultFy@users.noreply.github.com>
Signed-off-by: ThibaultFy <50656860+ThibaultFy@users.noreply.github.com>
Signed-off-by: ThibaultFy <50656860+ThibaultFy@users.noreply.github.com>
Signed-off-by: ThibaultFy <50656860+ThibaultFy@users.noreply.github.com>
Signed-off-by: ThibaultFy <50656860+ThibaultFy@users.noreply.github.com>
Signed-off-by: ThibaultFy <50656860+ThibaultFy@users.noreply.github.com>
Signed-off-by: ThibaultFy <50656860+ThibaultFy@users.noreply.github.com>
Signed-off-by: ThibaultFy <50656860+ThibaultFy@users.noreply.github.com>
@ThibaultFy ThibaultFy force-pushed the chore/bump-to-pydantic-2 branch from 011535a to 1110146 Compare September 7, 2023 08:29
@ThibaultFy ThibaultFy marked this pull request as ready for review September 7, 2023 08:36
@ThibaultFy ThibaultFy requested a review from a team as a code owner September 7, 2023 08:36
Copy link
Contributor

@SdgJlbl SdgJlbl left a comment

Choose a reason for hiding this comment

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

Thanks for the upgrade 🙏

setup.py Show resolved Hide resolved
@@ -1 +1 @@
__version__ = "0.47.0"
__version__ = "0.47.0.dev2"
Copy link
Contributor

Choose a reason for hiding this comment

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

Comment to change that in the merged version (I guess it's to get e2e tests passing)

substra/sdk/backends/local/backend.py Outdated Show resolved Hide resolved
substra/sdk/models.py Show resolved Hide resolved
substra/sdk/models.py Outdated Show resolved Hide resolved
substra/sdk/schemas.py Show resolved Hide resolved
substra/sdk/schemas.py Show resolved Hide resolved
substra/sdk/schemas.py Show resolved Hide resolved
Signed-off-by: ThibaultFy <50656860+ThibaultFy@users.noreply.github.com>
@ThibaultFy
Copy link
Member Author

/e2e --refs substrafl=chore/bump-to-pydantic-2,substra=chore/bump-to-pydantic-2,substra-tests=chore/bump-to-pydantic-2,substra-backend=chore/bump-to-pydantic-2 --tests sdk,substrafl --benchmarks mnist --mode standalone

@Owlfred
Copy link

Owlfred commented Sep 7, 2023

End to end tests: ✔️ SUCCESS

Awesome sauce!

Signed-off-by: ThibaultFy <50656860+ThibaultFy@users.noreply.github.com>
@ThibaultFy ThibaultFy requested a review from SdgJlbl September 7, 2023 14:10
@@ -602,7 +602,10 @@ def _output_from_spec(outputs: Dict[str, schemas.ComputeTaskOutputSpec]) -> Dict


def _schemas_list_to_models_list(inputs: Any, model: Any) -> Any:
return [model.model_validate(input_schema.model_dump()) for input_schema in inputs]
if inputs:
Copy link
Contributor

Choose a reason for hiding this comment

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

can inputs be None?
If it's always an iterable, you don't need the conditional

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, for instance in TaskSpec, inputs is given as "optional". In practice, it does not looks lite to be none sometimes but I added the condition to match the typing

Copy link
Contributor

Choose a reason for hiding this comment

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

minor but I feel the flow is slightly more readable with

if not inputs: 
    return []
return [model.model_validate(input_schema.model_dump()) for input_schema in inputs]

(reduce nested statements)

Copy link
Contributor

@SdgJlbl SdgJlbl left a comment

Choose a reason for hiding this comment

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

Thanks, don't forget to adjust the version number

Signed-off-by: ThibaultFy <50656860+ThibaultFy@users.noreply.github.com>
Signed-off-by: ThibaultFy <50656860+ThibaultFy@users.noreply.github.com>
@ThibaultFy ThibaultFy merged commit 08c99ad into main Sep 8, 2023
@ThibaultFy ThibaultFy deleted the chore/bump-to-pydantic-2 branch September 8, 2023 14:00
ThibaultFy added a commit to Substra/substra-backend that referenced this pull request Sep 8, 2023
# Companion PR

- Substra/substra#375 (main)
- Substra/substrafl#159
- Substra/substra-tests#272
- #718

---------

Signed-off-by: ThibaultFy <50656860+ThibaultFy@users.noreply.github.com>
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