added: exclusion violation custom error #3200

manthan-jsharma · 2023-09-08T18:15:56Z

Fixes #2126

Technical details

Screenshots

Checklist

My pull request has a descriptive title (not a vague title like Update index.md).
My pull request targets the develop branch of the repository
My commit messages follow best practices.
My code follows the established code style of the repository.
I added tests for the changes I made (if applicable).
I added or updated documentation (if applicable).
I tried running the project locally and verified that there are no
visible errors.

Developer Certificate of Origin

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

dmos62

Looking better! Yes, you want a test for this. I wouldn't bother using a CSV file. Create the fixture you need via psycopg calls. Read other fixtures for inspiration.

seancolsen · 2023-11-01T19:13:15Z

@manthan-jsharma Are you still planning to work on this?

manthan-jsharma · 2023-11-06T08:24:07Z

Certainly, I will be completing it. Currently, I am in the process of understanding how to import CSV files for the test cases. It would be helpful if you could provide guidance on this matter. Apart from that, the work is nearly finished.

seancolsen · 2023-11-06T15:41:21Z

@manthan-jsharma

It would be helpful if you could provide guidance on this matter

If you need help, please ask specific questions, taking care to explain exactly what problem you're having. Be clear about what steps you've taken, what expectation you have, and what observations you've made. If your questions are specific to the changes in this PR or are about implementation strategy, then the best place to ask is within this PR thread. However if your questions are more general questions about development processes and workflows, then it's probably better to ask in Matrix

…nstraints

manthan-jsharma · 2023-11-12T11:06:53Z

Hello @seancolsen and @dmos62,

I have implemented the custom message for the exclusion violation, added room_reservation.csv for the test case, and made an effort to include the corresponding test case. However, I'm currently facing challenges in writing the class for the ExclusionViolation constraint, and this is the only remaining part.

The code for the test case has been pushed in this commit: Link to the commit.

@seancolsen and @dmos62, could you please guide me on how to write the expression for the exclusion constraint in this section of the code: Link to the code? Additionally, I am unsure if it is even possible to add the exclusion constraint using SQLAlchemy version 1.4.26. I couldn't find relevant information in the documentation; here is the link to the documentation: SQLAlchemy Documentation.

seancolsen · 2023-11-13T13:17:40Z

@manthan-jsharma, @dmos62 will no longer be working on this ticket. I'm re-assigning this ticket to @Anish9901 who I hope will be able to answer your questions.

Anish9901

Hey @manthan-jsharma, I have requested some changes and addressed your queries in the code review please take a look at the specific comments.

Anish9901 · 2023-11-15T11:26:07Z

db/constraints/base.py

@@ -64,3 +64,20 @@ def get_constraint_def_json(self):
                }
            ]
        )
+
+
+class ExclusionViolation(Constraint):


This class won't be necessary for now, the constraint classes that are defined in this file are the ones that we support , meaning, we allow DDL operations like CREATE & DROP and DQL operations like SELECT. The issue you are solving is not to add support for this constraint but to handle any errors properly if violation for this constraint does occur on a table which has EXCLUSION constraint on it.

I've removed the class

Anish9901 · 2023-11-15T12:13:02Z

mathesar/api/exceptions/database_exceptions/exceptions.py

+            except TypeError:
+                details = {
+                    "constraint": None,
+                }


When does this occur?

I've just handled an edge case. This code will only execute in the event of a failure within the try block.

Shouldn't that be handled using Exception as it a base class for all the python exceptions instead of TypeError? The TypeError you have used here is a python type error and not a database type error.

Anish9901 · 2023-11-15T12:21:20Z

mathesar/api/exceptions/database_exceptions/exceptions.py

+        exception_detail = get_default_exception_detail(
+            exception, self.error_code, message, field, details
+        )._asdict()
+        self.detail = [exception_detail]
+        self.status_code = status_code


use super() instead.

This is done, I've used super here

Anish9901 · 2023-11-15T12:31:39Z

mathesar/tests/api/test_record_api.py

+    # table.add_constraint(
+    #     ExclusionViolation(
+    #     what parameters should i pass ?
+    #     )
+    # )


You could just use raw SQL to add the required constraint for now.

I've attempted several SQL queries, but none of them seems to be working successful. Additionally, I couldn't find any examples in the documentation on how to add exclusion constraints using SQL. Could you please provide guidance on adding exclusion constraints in SQL here @Anish9901

Try something similar to the SQL provided in the issue(#2126)

As discussed, i am sharing the solution but it did not work.

tried the query by directly using conn.execute, but got an error that TSRANGE is not defined.

tried by not using text from sqlalchemy but got an internal error from sqlalchemy library.

You are facing this problem because the data type of the columns of the CSV are getting inferred as TEXT instead of DATE

Anish9901 · 2023-11-15T12:32:27Z

mathesar/tests/api/test_record_api.py

@@ -1464,3 +1464,33 @@ def test_record_patch_unique_violation(create_patents_table, client):
        f'/api/db/v0/tables/{table.id}/constraints/{constraint_id}/'
    ).json()
    assert actual_constraint_details['name'] == 'NASA unique record PATCH_pkey'
+
+
+def test_record_patch_exclusion_violation(room_reservations_table, client):


Please create a test case for POST as well.

Anish9901 · 2023-12-04T23:36:33Z

@manthan-jsharma I've added 12aa7e8 that takes care of the issues you were facing and the commit also include some general cleanups. Let me know if you have any concerns.

Outdated

manthan-jsharma added 2 commits September 8, 2023 23:39

added: exclusion violation custom error

5771546

Merge branch 'develop' into issue-2126

6403cb6

This comment was marked as outdated.

Sign in to view

manthan-jsharma and others added 5 commits September 14, 2023 11:07

Merge branch 'develop' into issue-2126

4925295

Merge branch 'develop' into issue-2126

8d8616a

added : default variable for exclusion violation msg

2c78888

Merge branch 'develop' into issue-2126

f19e70a

Merge branch 'develop' into issue-2126

b7b3d45

rajatvijay assigned manthan-jsharma Sep 22, 2023

rajatvijay added the pr-status: revision A PR awaiting follow-up work from its author after review label Sep 22, 2023

rajatvijay added this to the Backlog milestone Sep 22, 2023

This comment was marked as resolved.

Sign in to view

added: default msg for exclusion violation

58dd459

manthan-jsharma force-pushed the issue-2126 branch from 42b7fb7 to 58dd459 Compare September 23, 2023 20:02

This comment was marked as resolved.

Sign in to view

fix : lint error

9000f93

dmos62 previously requested changes Oct 3, 2023

View reviewed changes

Merge branch 'develop' into issue-2126

7d89d10

manthan-jsharma added 2 commits November 12, 2023 15:58

added: room_reservation.csv for adding test case for exclusion for co…

10428d0

…nstraints

Merge branch 'develop' into issue-2126

677ee03

fix: linter error

5604013

seancolsen assigned Anish9901 and unassigned manthan-jsharma Nov 13, 2023

seancolsen added pr-status: review A PR awaiting review and removed pr-status: revision A PR awaiting follow-up work from its author after review labels Nov 13, 2023

Anish9901 requested changes Nov 15, 2023

View reviewed changes

Anish9901 added pr-status: revision A PR awaiting follow-up work from its author after review and removed pr-status: review A PR awaiting review labels Nov 15, 2023

manthan-jsharma and others added 4 commits November 28, 2023 12:06

removed: ExclusionViolation class from base.py

7b601ab

added: super in ExclusionViolationAPIException class

3771bcb

Merge branch 'develop' into issue-2126

42a1b3a

cleanups & fix tests

12aa7e8

Anish9901 enabled auto-merge December 4, 2023 23:39

Anish9901 approved these changes Dec 4, 2023

View reviewed changes

Anish9901 added this pull request to the merge queue Dec 4, 2023

Merged via the queue into mathesar-foundation:develop with commit 1644ec5 Dec 4, 2023
21 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

added: exclusion violation custom error #3200

added: exclusion violation custom error #3200

manthan-jsharma commented Sep 8, 2023 •

edited

This comment was marked as outdated.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

dmos62 left a comment •

edited

seancolsen commented Nov 1, 2023

manthan-jsharma commented Nov 6, 2023

seancolsen commented Nov 6, 2023

manthan-jsharma commented Nov 12, 2023

seancolsen commented Nov 13, 2023

Anish9901 left a comment

Anish9901 Nov 15, 2023

manthan-jsharma Nov 28, 2023

Anish9901 Nov 15, 2023

manthan-jsharma Nov 28, 2023

Anish9901 Nov 28, 2023 •

edited

Anish9901 Nov 15, 2023

manthan-jsharma Nov 28, 2023

Anish9901 Nov 15, 2023

manthan-jsharma Nov 28, 2023

Anish9901 Nov 28, 2023

manthan-jsharma Dec 4, 2023

Anish9901 Dec 4, 2023

Anish9901 Nov 15, 2023

Anish9901 commented Dec 4, 2023

added: exclusion violation custom error #3200

added: exclusion violation custom error #3200

Conversation

manthan-jsharma commented Sep 8, 2023 • edited

Checklist

Developer Certificate of Origin

This comment was marked as outdated.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

dmos62 left a comment • edited

Choose a reason for hiding this comment

seancolsen commented Nov 1, 2023

manthan-jsharma commented Nov 6, 2023

seancolsen commented Nov 6, 2023

manthan-jsharma commented Nov 12, 2023

seancolsen commented Nov 13, 2023

Anish9901 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Anish9901 Nov 28, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Anish9901 commented Dec 4, 2023

manthan-jsharma commented Sep 8, 2023 •

edited

dmos62 left a comment •

edited

Anish9901 Nov 28, 2023 •

edited