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

Error when editing the schema of the resource #467

Closed
pdelboca opened this issue Jul 23, 2024 · 7 comments · Fixed by #493
Closed

Error when editing the schema of the resource #467

pdelboca opened this issue Jul 23, 2024 · 7 comments · Fixed by #493
Assignees
Labels
bug Something isn't working

Comments

@pdelboca
Copy link
Member

Overview

When editing the schema of a resource, the application raises an error due to a missing table in the database. (The table that holds the dump of the file)

  File "/home/pdelboca/Repos/opendataeditor/server/endpoints/table/patch.py", line 34, in endpoint
    return action(request.app.get_project(), props)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/pdelboca/Repos/opendataeditor/server/endpoints/table/patch.py", line 105, in action
    resource.write_table(path=str(target))

.....

  File "/home/pdelboca/Repos/opendataeditor/.python/opendataeditor/lib/python3.11/site-packages/frictionless/formats/sql/adapter.py", line 66, in read_schema
    table = self.metadata.tables[table_name]
            ~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^
KeyError: 'mini_csv_3'

Steps to reproduce

  1. Navigate to a resource
  2. Go to Metadata
  3. Edit a value (like Title)
  4. Click on save button
simplescreenrecorder-2024-07-23_09.25.06.webm

Technical notes

The problem is that inside /table/patch, the method helpers.patch_record() deletes the table of the resource from the database and then when trying to execute resource.write_table(path=str(target)) it fails due to a missing table.

@pdelboca
Copy link
Member Author

@roll could you take a look at this issue? It is not clear what are the responsibilities of the patch_record functions or why are we deleting the table when editing the metadata.

There is a smelly comment in that part of the code which suggest we can improve it, but there is no context on why that code is there or why it is needed:

    # Clear database
    # TODO: use smarter logic to delete only if needed
    if updated and not toPath:
        delete_record(project, path=path, onlyFromDatabase=True)

@pdelboca pdelboca added the bug Something isn't working label Jul 23, 2024
@roll
Copy link
Collaborator

roll commented Aug 7, 2024

@pdelboca
I think the real fix to this messy situation with data/metadata editing would be separating data and metadata editing logic completely (having separated endpoints like table_update and record/resource_update). It will be possible if the metadata editing UI is moved to a standalone dialog instead of being a "panel" (making it a separate workflow "Open Metadata -> Edit Metadata -> Save Metadata"). @Faithkenny WDYT?

@Faithkenny
Copy link
Collaborator

Good idea, @roll. Asides from a nice case like the ability to change the Column titles in the Metadata schema below (while seeing the change in real time in the split above), a Panel UI style isn't what I'd expect for an app primarily for non-technical users.

A Dialog workflow is definitely one approach to consider once I get to this section of the app

@pdelboca
Copy link
Member Author

pdelboca commented Aug 8, 2024

@roll could you provide more context on why is this happening?

I agree that data/metadata logic should be separated, I'm just wondering if this is an error that can be handled under the current logic instead of rewriting it.

@roll
Copy link
Collaborator

roll commented Aug 8, 2024

@pdelboca
Sure, I'll investigate tomorrow

@roll
Copy link
Collaborator

roll commented Aug 9, 2024

The fix - #493, and the issue describing more fundamental improvements to avoid similar issues in the future - #494

@pdelboca
Copy link
Member Author

Thanks @roll ! I will be adding #494 to the Metadata Sprints, maybe we can tackle it after the preview release of September.

@roll roll closed this as completed in #493 Aug 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants