Re: [PR] feat: add create_collection function with unit tests [airflow]
Lee-W commented on code in PR #50518: URL: https://github.com/apache/airflow/pull/50518#discussion_r2097162169 ## providers/mongo/src/airflow/providers/mongo/hooks/mongo.py: ## @@ -225,6 +225,39 @@ def get_collection(self, mongo_collection: str, mongo_db: str | None = None) -> return mongo_conn.get_database(mongo_db).get_collection(mongo_collection) +def create_collection( +self, +mongo_collection: str, +mongo_db: str | None = None, +create_if_exists: bool = True, +**create_kwargs: dict[str, Any], Review Comment: Sounds good. Thanks for trying -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] feat: add create_collection function with unit tests [airflow]
nailo2c commented on code in PR #50518: URL: https://github.com/apache/airflow/pull/50518#discussion_r2096948058 ## providers/mongo/src/airflow/providers/mongo/hooks/mongo.py: ## @@ -225,6 +225,39 @@ def get_collection(self, mongo_collection: str, mongo_db: str | None = None) -> return mongo_conn.get_database(mongo_db).get_collection(mongo_collection) +def create_collection( +self, +mongo_collection: str, +mongo_db: str | None = None, +create_if_exists: bool = True, +**create_kwargs: dict[str, Any], Review Comment: Can't narrow it further. `create_collection()` accepts kwargs like `read_preference: Optional[_ServerMode]`, `write_concern: Optional[WriteConcern]`, `read_concern: Optional[ReadConcern]`, etc. So without `Any`, linter will complain about missing types. Here are the errors I encountered when I assigned the type as `CodecOptions[Never] | None`. ```bash providers/mongo/src/airflow/providers/mongo/hooks/mongo.py:253: error: Argument 2 to "create_collection" of "Database" has incompatible type "**dict[str, Optional[CodecOptions[NoReturn]]]"; expected "Optional[_ServerMode]" [arg-type] db.create_collection(mongo_collection, **create_kwargs) ^ providers/mongo/src/airflow/providers/mongo/hooks/mongo.py:253: error: Argument 2 to "create_collection" of "Database" has incompatible type "**dict[str, Optional[CodecOptions[NoReturn]]]"; expected "Optional[WriteConcern]" [arg-type] db.create_collection(mongo_collection, **create_kwargs) ^ providers/mongo/src/airflow/providers/mongo/hooks/mongo.py:253: error: Argument 2 to "create_collection" of "Database" has incompatible type "**dict[str, Optional[CodecOptions[NoReturn]]]"; expected "Optional[ReadConcern]" [arg-type] db.create_collection(mongo_collection, **create_kwargs) ^ providers/mongo/src/airflow/providers/mongo/hooks/mongo.py:253: error: Argument 2 to "create_collection" of "Database" has incompatible type "**dict[str, Optional[CodecOptions[NoReturn]]]"; expected "Optional[ClientSession]" [arg-type] db.create_collection(mongo_collection, **create_kwargs) ^ providers/mongo/src/airflow/providers/mongo/hooks/mongo.py:253: error: Argument 2 to "create_collection" of "Database" has incompatible type "**dict[str, Optional[CodecOptions[NoReturn]]]"; expected "Optional[bool]" [arg-type] db.create_collection(mongo_collection, **create_kwargs) ^ Found 5 errors in 1 file (checked 1 source file) Error 1 returned ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] feat: add create_collection function with unit tests [airflow]
Lee-W commented on code in PR #50518: URL: https://github.com/apache/airflow/pull/50518#discussion_r2096682718 ## providers/mongo/src/airflow/providers/mongo/hooks/mongo.py: ## @@ -225,6 +225,39 @@ def get_collection(self, mongo_collection: str, mongo_db: str | None = None) -> return mongo_conn.get_database(mongo_db).get_collection(mongo_collection) +def create_collection( +self, +mongo_collection: str, +mongo_db: str | None = None, +create_if_exists: bool = True, +**create_kwargs: dict[str, Any], Review Comment: then what about changing into `CodecOptions[Never] | None`? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] feat: add create_collection function with unit tests [airflow]
nailo2c commented on code in PR #50518: URL: https://github.com/apache/airflow/pull/50518#discussion_r2096139374 ## providers/mongo/src/airflow/providers/mongo/hooks/mongo.py: ## @@ -225,6 +225,39 @@ def get_collection(self, mongo_collection: str, mongo_db: str | None = None) -> return mongo_conn.get_database(mongo_db).get_collection(mongo_collection) +def create_collection( +self, +mongo_collection: str, +mongo_db: str | None = None, +create_if_exists: bool = True, +**create_kwargs: dict[str, Any], Review Comment: Hi @Lee-W, I found that if I change the type to `dict[str, Any]`, an error occurs when I run pre-commit locally. So let me roll it back to type `Any` error ``` providers/mongo/src/airflow/providers/mongo/hooks/mongo.py:253: error: Argument 2 to "create_collection" of "Database" has incompatible type "**dict[str, dict[str, Any]]"; expected "Optional[CodecOptions[Never]]" [arg-type] db.create_collection(mongo_collection, **create_kwargs) ^ ``` the command I used ``` pre-commit run --files providers/mongo/src/airflow/providers/mongo/hooks/mongo.py ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] feat: add create_collection function with unit tests [airflow]
nailo2c commented on code in PR #50518: URL: https://github.com/apache/airflow/pull/50518#discussion_r2096121873 ## providers/mongo/src/airflow/providers/mongo/hooks/mongo.py: ## @@ -225,6 +225,39 @@ def get_collection(self, mongo_collection: str, mongo_db: str | None = None) -> return mongo_conn.get_database(mongo_db).get_collection(mongo_collection) +def create_collection( +self, +mongo_collection: str, +mongo_db: str | None = None, +create_if_exists: bool = True, Review Comment: good suggestion, it makes more sense -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] feat: add create_collection function with unit tests [airflow]
Lee-W commented on code in PR #50518:
URL: https://github.com/apache/airflow/pull/50518#discussion_r2095786852
##
providers/mongo/src/airflow/providers/mongo/hooks/mongo.py:
##
@@ -225,6 +225,39 @@ def get_collection(self, mongo_collection: str, mongo_db:
str | None = None) ->
return
mongo_conn.get_database(mongo_db).get_collection(mongo_collection)
+def create_collection(
+self,
+mongo_collection: str,
+mongo_db: str | None = None,
+create_if_exists: bool = True,
+**create_kwargs: Any,
+) -> MongoCollection:
+"""
+Create the collection (optionally a time‑series collection) and return
it.
+
+
https://pymongo.readthedocs.io/en/stable/api/pymongo/database.html#pymongo.database.Database.create_collection
+
+:param mongo_collection: Name of the collection.
+:param mongo_db: Target database; defaults to the schema in the
connection string.
+:param create_if_exists: If True and the collection already exists,
return it instead of raising.
+:param create_kwargs: Additional keyword arguments forwarded to
``db.create_collection()``,
+ e.g. ``timeseries={...}``, ``capped=True``.
+"""
+from pymongo.errors import CollectionInvalid
Review Comment:
I think this might not be an expensive import đŸ¤” so we probably can move it
to the top
##
providers/mongo/src/airflow/providers/mongo/hooks/mongo.py:
##
@@ -225,6 +225,39 @@ def get_collection(self, mongo_collection: str, mongo_db:
str | None = None) ->
return
mongo_conn.get_database(mongo_db).get_collection(mongo_collection)
+def create_collection(
+self,
+mongo_collection: str,
+mongo_db: str | None = None,
+create_if_exists: bool = True,
+**create_kwargs: Any,
Review Comment:
```suggestion
**create_kwargs: dict[str, Any],
```
nit
##
providers/mongo/src/airflow/providers/mongo/hooks/mongo.py:
##
@@ -225,6 +225,39 @@ def get_collection(self, mongo_collection: str, mongo_db:
str | None = None) ->
return
mongo_conn.get_database(mongo_db).get_collection(mongo_collection)
+def create_collection(
+self,
+mongo_collection: str,
+mongo_db: str | None = None,
+create_if_exists: bool = True,
Review Comment:
```suggestion
return_if_exists: bool = True,
```
should it be `return_if_exists` instead
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] feat: add create_collection function with unit tests [airflow]
potiuk closed pull request #50518: feat: add create_collection function with unit tests URL: https://github.com/apache/airflow/pull/50518 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] feat: add create_collection function with unit tests [airflow]
potiuk commented on PR #50518: URL: https://github.com/apache/airflow/pull/50518#issuecomment-2875903936 Closed / reopened to rebuild after rebase to see if static check still fails -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
