refactor: add __eq__ on Table

[aarcex3]

Summary

Implemented the __eq__ method on the Table class to address the issue explained in #1075

Closes

#1075

PD: I would like to check the implementation first, that's why this is a DRAFT. Also I couldn't setup the project for testing, what I mean by that is, I tried using docker to run postgres and cockroach, but no luck. Am I missing something?

Also in which file in tests/table would it be best to add tests for this implementation?

Thanks

[sinisaos]

@aarcex3 Thank you for your effort, but I think your solution is not correct. Sorry if I'm wrong. If you have this example

from piccolo.apps.user.tables import BaseUser
from piccolo_conf import DB

class Project(Table, db=DB):
    name = Varchar(length=100)
    owner = ForeignKey(BaseUser)

project = Project.objects(Project.owner).get(Project._meta.primary_key == 1).run_sync()
owner = BaseUser.objects().get(BaseUser._meta.primary_key == 1).run_sync()

print(project.owner == owner)
#  result is "piccolo_user"."id" = "piccolo_user"."id" and should be True

The solution from issue is better and the result is correct, but mypy complains. I think the solution should be something like this using getattr (Both results are correct and mypy is happy)

def __eq__(self, other) -> bool:
    return isinstance(other, Table) and getattr(
        self, self._meta.primary_key._meta.name, None
    ) == getattr(other, other._meta.primary_key._meta.name, None)

PD: I would like to check the implementation first, that's why this is a DRAFT. Also I couldn't setup the project for testing, what I mean by that is, I tried using docker to run postgres and cockroach, but no luck. Am I missing something?

I have Postgres and Cockroach installed on my local machine (for testing). I don't use Docker for testing so not much help from me. If you need help installing and using Cockroach locally, just ask I'll be glad to help.

Also in which file in tests/table would it be best to add tests for this implementation?

I would write the test here because it makes the most sense to me. Sorry for long comment.

[aarcex3]

Hi @sinisaos, thank you so much for the feedback!!.
I will give it a try and let you know.

[dantownsend]

Yeah, the solution that @sinisaos looks right.

When testing, I just run Postgres locally (i.e. not in a container). I use Postgres.app on the Mac, or just plain apt install postgresql on Ubuntu.

[sinisaos]

@aarcex3 For CockroachDB you can also install locally (use the installation for your OS). After installation (this is for a Linux machine, it should be similar on a Mac, I don't know about Windows)

1. Open a terminal and run CockroachDB (in single insecure node)

cockroach start-single-node --insecure

2. Open another terminal and use this command to create the database

cockroach sql --insecure --execute="DROP DATABASE IF EXISTS piccolo CASCADE;CREATE DATABASE piccolo;"

3. And finally you can start testing from piccolo directory

./scripts/test-cockroach.sh

[sinisaos]

@aarcex3 Were you able to run the test on your local machine? Here is the branch with code changes and tests. All the code is formatted and ready to merge (if it's good enough Daniel will merge it) and feel free to use it for this PR. To make sure the formatting and linter are satisfied, just run ./scripts/format.sh and ./scripts/lint.sh from piccolo directory. Thanks again for your interest in Piccolo. Hope this helps.

[aarcex3]

Hey @sinisaos, thank you so much for your help and guidance!!!. You didn't have to do all the work 😅 (I wanted to learn and try). But reading others people code is also useful 🤓.
Also, yes, I managed to run cockroach on my machine 🤩.

[sinisaos]

You didn't have to do all the work 😅 (I wanted to learn and try)

@aarcex3 Sorry, you're right. I just wanted to help, because I know when you're contributing to a project for the first time it can be frustrating. I won't make the same mistake again.

I ran the CI workflow and everything passed. Now you have to wait for @dantownsend to review or merge. Cheers