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.

Sign up for GitHub

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

Jump to bottom

chore(server/deps): Upgrade to pydantic ^2.4.2 [TCTC-7241] #1891

Merged
merged 23 commits into from
Dec 15, 2023
Merged

chore(server/deps): Upgrade to pydantic ^2.4.2 [TCTC-7241] #1891

merged 23 commits into from
Dec 15, 2023

Conversation

lukapeschke
Copy link
Contributor

No description provided.

@lukapeschke
chore: bump pydantic
c9d040a 
Signed-off-by: Luka Peschke <[email protected]>
@lukapeschke
chore: run bump-pydantic
2a607f8 
Signed-off-by: Luka Peschke <[email protected]>
@lukapeschke
fix remaining warnings
8bfc83a 
Signed-off-by: Luka Peschke <[email protected]>
@lukapeschke
fix tests
b68e22f 
Signed-off-by: Luka Peschke <[email protected]>
@lukapeschke
Stop using deprectaed pydantic methods
fdd5af8 
Signed-off-by: Luka Peschke <[email protected]>
@lukapeschke lukapeschke added need review dependencies Pull requests that update a dependency file DO NOT MERGE labels Sep 12, 2023
@lukapeschke lukapeschke self-assigned this Sep 12, 2023
@render
Copy link

render bot commented Sep 12, 2023

Your Render PR Server URL is https://weaverbird-playground-pr-1891.onrender.com.

Follow its progress at https://dashboard.render.com/web/srv-ck08jrnhdsdc73dcv920.

@lukapeschke
fix lockfile
f48d92e 
Signed-off-by: Luka Peschke <[email protected]>
@lukapeschke
Copy link
Contributor Author

depends on ToucanToco/toucan-connectors#1272

PrettyWood
PrettyWood requested changes Sep 28, 2023
View reviewed changes
Copy link
Member

@PrettyWood PrettyWood left a comment

Choose a reason for hiding this comment

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

Mini remarque mais LGTM dans l'ensemble !
J'aurais bien rajouté qqes TUs pour les endroits où t'as changé du code pour faire du damage control. Pas 100% confiant qu'on laisse pas passer des trucs

server/pyproject.toml Outdated
@@ -27,14 +27,16 @@ Quart-CORS = {version = ">=0.5,<0.7", optional = true}
hypercorn = {version = ">=0.13,<0.15", optional = true}
pymongo = {version = ">=4.2.0", optional = true, extras = ["srv", "tls"]}
psycopg = {optional = true, version = "^3.0.15"}
toucan-connectors = {version = "^4.5.1", optional = true, extras = ["google_big_query", "mongo", "Redshift", "snowflake", "awsathena", "mysql"]}
# toucan-connectors = {version = "^4.5.1", optional = true, extras = ["google_big_query", "mongo", "Redshift", "snowflake", "awsathena", "mysql"]}
toucan-connectors = {git = "https://github.com/ToucanToco/toucan-connectors", branch = "upgrade-pydantic", optional = true, extras = ["google_big_query", "mongo", "Redshift", "snowflake", "awsathena", "mysql"]}
Copy link
Member

Choose a reason for hiding this comment

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

pourquoi c'est une double dep ? (prod et dev)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

En prod c'est pour le playground, uniquement via l'extra playground. En dev c'est pour tirer les dépendances de connecteurs nativesql et pour avoir accès à nosql_apply_parameters_to_query

Copy link
Member

@PrettyWood PrettyWood Sep 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

right j'avais pas vu le playground merci

server/pyproject.toml Outdated
@@ -13,7 +13,7 @@ readme = "README.md"

[tool.poetry.dependencies]
python = ">=3.10, <3.12"
pydantic = "^1.9.1"
pydantic = "^2.3.0"
Copy link
Member

Choose a reason for hiding this comment

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

on peut mettre 2.4.1 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

0908533

server/src/weaverbird/pipeline/steps/utils/combination.py Outdated
Comment on lines 27 to 30
# FIXME: pydantic v2 is stricter with typing, and requires this to be
# list[dict] | str | PipelineStep. However it would result in a circular import.
# In python 3.11, importing the PipelineStep only when TYPE_CHECKING is True works,
# but seems a bit hacky. So this ensures that the input can only be a dict
Copy link
Member

Choose a reason for hiding this comment

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

why not ca me derangeait pas le truc avant

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ça pète en python 3.10

Copy link
Member

Choose a reason for hiding this comment

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

arf

server/src/weaverbird/backends/pandas_executor/steps/utils/dates.py Outdated
Comment on lines 29 to 30
relative_date.date
if isinstance(relative_date.date, datetime)
Copy link
Member

Choose a reason for hiding this comment

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

pq ca marchait avant ? Ca coercait de base un str en datetime vu qu'on avait datetime | str alors que mtt non car la v2 est plus stricte ?
Si oui on voudrait pas plutôt changer dans RelativeDate et mettre que le type datetime avec un validator ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes c'est ça. Si ça ferait plus de sens mais je voulais être le plus conservateur possible dans le comportement. Mais tu as raison, quitte a faire un breaking change, autant faire la modif maintenant

Copy link
Contributor Author

Choose a reason for hiding this comment

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

9e0875b

@lukapeschke lukapeschke changed the title chore(server/deps): Upgrade to pydantic ^2.3.0 chore(server/deps): Upgrade to pydantic ^2.4.2 [TCTC-7241] Nov 22, 2023
@lukapeschke
updated connectors branch
3210283 
Signed-off-by: Luka Peschke <[email protected]>
@lukapeschke
fix: Add missing python-dateutil dependency
1b1f9f0 
Signed-off-by: Luka Peschke <[email protected]>
@lukapeschke
Merge branch 'master' into upgrade-pydantic
186a0bb
@lukapeschke
fix: forbid PipelineStepWithRefs in PipelineWithVariables
6a62bf1 
Signed-off-by: Luka Peschke <[email protected]>
@lukapeschke
feat(server): drop Python 3.10 support
c0665f5 
Signed-off-by: Luka Peschke <[email protected]>
@lukapeschke
refactor(server/combination): Allow PipelineOrDomainName to be a list…
2582ed4 
…[PipelineStep]

Signed-off-by: Luka Peschke <[email protected]>
@lukapeschke
refactor(server): replace update_forward_refs() with model_rebuild()
c358026 
Signed-off-by: Luka Peschke <[email protected]>
@lukapeschke
refactor(server): Stronger typing on pipelineOrDomainName
052b713 
Signed-off-by: Luka Peschke <[email protected]>
@lukapeschke
Copy link
Contributor Author

Last commit includes a workaround for pydantic/pydantic#7487 in case of nested PipelineOrDomainName

@lukapeschke
fix PipelineWithRefs typing
69e8d9e 
Signed-off-by: Luka Peschke <[email protected]>
PrettyWood
PrettyWood reviewed Dec 7, 2023
View reviewed changes
server/src/weaverbird/pipeline/steps/utils/combination.py
@@ -17,8 +17,56 @@ def __hash__(self):
return hash(f"__ref__{self.uid}")


# FIXME: Required because of https://github.com/pydantic/pydantic/issues/7487.
# Repro case:
Copy link
Member

Choose a reason for hiding this comment

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

👍 vraiment cool d'avoir mis aussi le MRE

@lukapeschke
bump connectors
afe7a05 
Signed-off-by: Luka Peschke <[email protected]>
@lukapeschke lukapeschke merged commit 99346a8 into master Dec 15, 2023
1 check passed
@lukapeschke lukapeschke deleted the upgrade-pydantic branch December 15, 2023 11:58
@PrettyWood PrettyWood mentioned this pull request Dec 20, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PrettyWood PrettyWood PrettyWood approved these changes

Assignees

@lukapeschke lukapeschke

Labels
dependencies Pull requests that update a dependency file DO NOT MERGE
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lukapeschke @PrettyWood