dymmond / mongoz

ODM with Pydantic made it simple
https://mongoz.dymmond.com
BSD 3-Clause "New" or "Revised" License
19 stars 4 forks source link

With setting the indexes auto generate flag is true #35

Closed harshalizode closed 5 months ago

harshalizode commented 6 months ago

Hello,

I have a Model as:

indexes = [
    Index(keys=[("year", Order.DESCENDING), ("genre", IndexType.HASHED)]),
]

class AnotherMovie(Document):
    name: str = mongoz.String()
    email: str = mongoz.Email(index=True, unique=True)
    year: int = mongoz.Integer()
    uuid: Optional[ObjectId] = mongoz.ObjectId(null=True)

    class Meta:
        registry = client
        indexes = indexes
        database = "test_db"
        autogenerate_index = True

When I create a duplicate record in this model, it allows me to do so, unless I use the await AnotherMovie.create_indexes()method.

And if any model does not have indexes, calling the await AnotherMovie.create_indexes() method returns an error as well.

Thank you.

tarsil commented 6 months ago

Hmmm. Interesting. I will have a look and report back to you. Something in the auto generate indexes seems to fail

harshalizode commented 6 months ago

Okay, Thank you for your quick response.

harshalizode commented 6 months ago

Hello, Is this issue resolved?

tarsil commented 6 months ago

It should be released next week. Apologies but we had some hectic weeks before

harshalizode commented 6 months ago

Okay, Thank you for reply.

tarsil commented 6 months ago

@harshalizode this should be addressed this week btw

harshalizode commented 6 months ago

Okay, thanks for acknowledging me.

tarsil commented 6 months ago

Okay, thanks for acknowledging me.

You are always acknowledged 🙂. You deserve all the attention. We just had a lot of work and some issues with the release API but it's being addressed.

harshalizode commented 6 months ago

Thanks for your attention, is this issue is fixed in the latest version?

harshalizode commented 5 months ago

Hello, Auto generation index issue is not fixed yet? We having the same issue after enabled the autogenerate_index=True

Any updates regarding it?

tarsil commented 5 months ago

@harshalizode apologies as I've been extremely busy but it's being addressed

harshalizode commented 5 months ago

Okay, thank you.

tarsil commented 5 months ago

This is very odd. I've ran the tests and it is passing. Can you give me some details how to assemble it, please?

harshalizode commented 5 months ago

Hello, Yes, I have checked this issue it is creating the indexes in the database that we have provided in the Meta class. If I want to create the index in the other database that not registerd in the Meta, that need to check once how can we done in the Project. Do you know how to solve it? It has to do with the issue of creating support for multi tenants. I want to make an index in each database. Thank you.

tarsil commented 5 months ago

@harshalizode I thought so it could be that. So, what I would need from you if you don't mint it's an example how were you doing this so I can try to replicate and implement some solution on top.

Is that ok?

tarsil commented 5 months ago

How is the design for that? Using the using? It would be great to have an example. Although we can probably provide a function that allows the creation of indexes on multiple dbs. The autogenerate_index works for the declared table in the metadata only. For this case, we would probably need something else.

tarsil commented 5 months ago

@harshalizode added a new feature for that like this (see tests below).

I can release this if you are happy with it.

async def test_model_using() -> None:
    await Movie.create_indexes_for_multiple_databases(["test_my_db", "test_second_db"])

    await Movie.objects.create(name="Mongoz", email="mongoz@mongoz.com", year=2023)
    await Movie.objects.using("test_my_db").create(
        name="Mongoz", email="mongoz@mongoz.com", year=2023
    )

    await Movie.objects.using("test_second_db").create(
        name="Mongoz", email="mongoz@mongoz.com", year=2023
    )
    with pytest.raises(DuplicateKeyError):
        await Movie.objects.create(name="Mongoz", email="mongoz@mongoz.com", year=2023)

    with pytest.raises(DuplicateKeyError):
        await Movie.objects.using("test_my_db").create(
            name="Mongoz", email="mongoz@mongoz.com", year=2023
        )
    with pytest.raises(DuplicateKeyError):
        await Movie.objects.using("test_second_db").create(
            name="Mongoz", email="mongoz@mongoz.com", year=2023
        )
harshalizode commented 5 months ago

Yes, releasing it will help a lot. One more thing: when should we run this logic as a startup script or any other solution for it? since once the index is created, we don't need to create it again. Do you have any ideas for a productive fix? Thank you.

tarsil commented 5 months ago

I would advise to run it in a script as one off. This is provided to help you but like you said, once it's done, you don't need it anymore

harshalizode commented 5 months ago

Yes, but it has the issue if we have updated the model or unique constraint then it need to run it manually , because we don't have the feature has migration as Django.

tarsil commented 5 months ago

Of course not. Django is ORM and schema oriented and Mongoz is NoSQL and no schemas, therefore no migrations. I can try to find a way to help you out with that

harshalizode commented 5 months ago

Yes, thank you. I will also check the productive solution and let you know for the release.

tarsil commented 5 months ago

Ok, open to suggestions on that

tarsil commented 5 months ago

@harshalizode I believe I was able to come out with a clean and effective solution for this. A new release 0.9.0 came out already with that.

https://mongoz.dymmond.com/release-notes/

harshalizode commented 5 months ago

@tarsil I appreciate the release. The document_checks and check_indexes methods are excellent for determining when index creation is necessary since they allow us to see whether the indexes have been updated or need to be updated. Thank you.

tarsil commented 5 months ago

@harshalizode Of course 👍🏼 any time. Thank you for asking this. I will be closing this issue since it was addressed.