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

Stop misusing TableMeta.dataset reference and make it read-only #1274

Closed
Marigold opened this issue Jun 27, 2023 · 4 comments
Closed

Stop misusing TableMeta.dataset reference and make it read-only #1274

Marigold opened this issue Jun 27, 2023 · 4 comments

Comments

@Marigold
Copy link
Collaborator

TableMeta.dataset should be a reference to DatasetMeta, but it's currently a cached copy that might be overwritten. This has caused confusion and we had to implement weird hacks to make it behave.

It's misused in several places if you search for metadata.dataset = (probably doing nothing as it gets overwritten when saving the dataset).

Instead of keeping a copy, we could make a read-only property out of it and even point it to the actual DatasetMeta object.

We can also consider DRYing dataset metadata that is saved in both index.json file and [table].meta.json files (and load dataset's metadata when loading the table).

@larsyencken
Copy link
Collaborator

This PR adds the unit test for this situation: #1276

@larsyencken
Copy link
Collaborator

Calling this one done after the merged changes.

@Marigold
Copy link
Collaborator Author

Marigold commented Aug 2, 2023

This has been fixed in ETL steps, but it's still a problem in core ETL. It has caused some nasty bugs in the past. I'm not going to reopen this now, but I might if I run into this again in the future.

@larsyencken
Copy link
Collaborator

It will fail unit tests if you do this now, which in some sense is good enough to solve this specific issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants