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

Add "using" method in model queryset manager, which will be used to change database at runtime while querying. #28

Merged
merged 7 commits into from
May 2, 2024

Conversation

harshalizode
Copy link
Contributor

The "using" method allows us to change the database at runtime while querying. If the user does not use the "using" method, it behaviour the same as before; however, I have included logic to change the database during runtime.

It is compatible with create, get, all, filter, with the exception of the update query, which needs to be modified in order to be used with the "using" method because it creates duplicate documents with the updated one.

We can use the "using" method as follows:

User.objects.using("DB_NAME").all()

…hange database at runtime while querying. And this method will work for create, get, filter querymanager's methods
@tarsil
Copy link
Collaborator

tarsil commented Apr 24, 2024

@harshalizode this is great! Thank you so much for this but I need to ask you a couple of things.

Can you update the docs to also have this using referenced there, please? Maybe in the queries.md?

Also, some tests to this would be great to have. Since it's mongo db, you can pass in the meta whatever you need.

Having all of this, we can release it today

@harshalizode
Copy link
Contributor Author

I have updated the doc and wrote test cases for it, and it passed.

@tarsil
Copy link
Collaborator

tarsil commented Apr 25, 2024

@harshalizode the linting is failing.

In your local machine run:

scripts/lint

This should fix the problems

@tarsil tarsil self-requested a review April 25, 2024 08:53
@harshalizode
Copy link
Contributor Author

Which linter is used in MongoZ?

@tarsil
Copy link
Collaborator

tarsil commented Apr 25, 2024

@harshalizode mongoz repo has everything you need. Here https://mongoz.tarsild.io/contributing/ contains what you need to do to run it locally.

Although there is a bug there I need to fix. its not scripts/format anymore but scripts/lint.

When you install the requirements and run those commands and pass, it should pass in the CI as well since it uses the same.

We use in the script ruff black and mypy. That is why its easier to run it after installing the requirements.

@tarsil
Copy link
Collaborator

tarsil commented Apr 26, 2024

@harshalizode if you can run the scripts/install + scripts/lint this errors should be easily addressed and you can push them.

It would be great to have this great feature in Mongoz.

@harshalizode
Copy link
Contributor Author

I have resolved the Linter issues.

@tarsil
Copy link
Collaborator

tarsil commented Apr 29, 2024

@harshalizode still complains? Did you run the scripts/lint?

Maybe you can also install the pre-commits and run it

@harshalizode
Copy link
Contributor Author

harshalizode commented Apr 29, 2024

I have resolved all the issues.
success_lint

@tarsil
Copy link
Collaborator

tarsil commented Apr 29, 2024

I have resolved all the issues. success_lint

I can see that but once you resolve them you need to still do a git add and git push for the CI to run.

You can see the error https://github.com/tarsil/mongoz/actions/runs/8875747891/job/24365837286?pr=28

@@ -19,7 +21,7 @@ class Document(DocumentRow):
Representation of an Mongoz Document.
"""

async def create(self: "Document") -> "Document":
async def create(self: "Document", collection: AsyncIOMotorCollection = None) -> "Document": # type: ignore
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe here it is better to

 collection: Union[AsyncIOMotorCollection, None] = None

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@tarsil
Copy link
Collaborator

tarsil commented Apr 29, 2024

Overall the PR looks good besides the minor changes requested 👍🏼

@tarsil
Copy link
Collaborator

tarsil commented Apr 29, 2024

@harshalizode the linting is still a problem.

This is what I would do.

  1. Install the pre-commit
  2. git init (no worries just to start the pre commit)
  3. scripts/lint
  4. git add
  5. git push

@harshalizode
Copy link
Contributor Author

Yes, after following the steps, I had two issues, which I resolved.

@tarsil tarsil merged commit 9393bd0 into dymmond:main May 2, 2024
6 checks passed
@tarsil
Copy link
Collaborator

tarsil commented May 2, 2024

@harshalizode congratulation! Your PR made it into the new release 0.8.0 :)

Thank you so much, I hope Mongoz and Esmerald are treating you well.

@harshalizode
Copy link
Contributor Author

Thanks for all your assistance :)

@tarsil
Copy link
Collaborator

tarsil commented May 2, 2024

Thanks for all your assistance :)

Anytime :). How is Esmerald and Mongoz working for you?

@harshalizode
Copy link
Contributor Author

Both Esmerald and Mongoz are nice, but when utilizing Esmerald with Mongoz, there are certain difficulties, such as:

  1. Extending a Document to override the QueryManager.
  2. MOGOZ.Date field has a parsing issue.
    We have identified a few problems and need to find a solution.

@tarsil
Copy link
Collaborator

tarsil commented May 2, 2024

@harshalizode ok good to know. Let's fix those then. Esmerald has nothing to do with that apparently. Do you still have the extending problem?

Can you provide me examples of all the issues you are currently facing?

This way we can address them

@harshalizode
Copy link
Contributor Author

Yes, there is still an issue with overriding the QueryManager.

I use the Abstract document as the BaseDocument by inheriting the mongoz.Document.
I have replaced the Quermanager with the CustomManager.

class BaseDocument(mongoz.Document):
    """
    The base document for all documents using the `registry` registry.
    """
    objects: ClassVar[CustomManger] = CustomManger()

    class Meta:
        """
            It contains the Meta information related the database for \
                each model level.
        """
        abstract = True
        registry = settings.client
        database = settings.db

Now, I'm trying to inherit the BaseDocument into my view as

class Employee(BaseDocument):
    month_period: int = mongoz.Integer(null=True)
    full_name: str = mongoz.String()
    employee_code: str = mongoz.String()
    date_of_joining: datetime.datetime = mongoz.DateTime(null=True)
    e_mail: str = mongoz.String(null=True)
    mobile_no: str = mongoz.String(null=True)

Whenever I try to query in MongoDB using the manager as Employee.objects.all()
Employee.objects returns me the Manager (Default Manager) and not the CustomManager.

That's why I'm overriding the manager at the model level.

class Employee(BaseDocument):
    month_period: int = mongoz.Integer(null=True)
    full_name: str = mongoz.String()
    employee_code: str = mongoz.String()
    date_of_joining: datetime.datetime = mongoz.DateTime(null=True)
    e_mail: str = mongoz.String(null=True)
    mobile_no: str = mongoz.String(null=True)

    objects: ClassVar[CustomManger] = CustomManger()

Thanks.

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.

2 participants