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

Ignorable tables #141

Merged
merged 7 commits into from
Sep 19, 2023
Merged

Ignorable tables #141

merged 7 commits into from
Sep 19, 2023

Conversation

mhauru
Copy link
Collaborator

@mhauru mhauru commented Sep 6, 2023

Add option to ignore tables. In so doing, also rework example_orm.py.

  • Add option in the config to ignore some table entirely
  • Start checking in the functional tests that the produced ORM is as expected. This allows testing the above feature.
  • For this purpose, completely rework example_orm.py so that it is now the valid output of make-tables on src.dump.
  • Make make-tables run black on the outputted code.

Closes #139

* Add option in the config to ignore some table entirely
* Start checking in the functional tests that the produced ORM is as
  expected. This allows testing the above feature.
* For this purpose, completely rework example_orm.py so that it is now
  the valid output of make-tables on src.dump.
* Make make-tables run black on the outputted code.

WIP because this commit does not pass mypy.
@mhauru mhauru requested a review from Iain-S September 6, 2023 17:01
@mhauru
Copy link
Collaborator Author

mhauru commented Sep 7, 2023

The tests pass for me locally. @Iain-S was this the same issue you already fixed in some other PR?

Some tables can't be dropped from the ORM file, namely ones to which
there is a foreign key referenc. We still want to be able to ignore
those tables when creating generators, deleting and creating data, etc.
@mhauru
Copy link
Collaborator Author

mhauru commented Sep 12, 2023

I ran into an issue where I some tables kept getting included in orm.py even though I set ignore: true. This was because one of the not-ignored tables had a foreign key reference to it. That makes sense, you then have to have that table in your orm.py, but we may still want to ignore that table for the purposes of creating generators, data, etc. I added a commit that implements this functionality.

@mhauru mhauru mentioned this pull request Sep 15, 2023
Copy link
Collaborator

@Iain-S Iain-S left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All looks good to me. I've pushed a couple of minor changes straight to this branch.

Comment on lines +481 to +497
metadata.reflect(
engine,
# The type-ignore is due to an erroneous type annotation in SQLAlchemy.
only=reflect_if, # type: ignore
)

for table_name in metadata.tables.keys():
table_config = tables_config.get(table_name, {})
ignore = table_config.get("ignore", False)
if ignore:
logging.warning(
"Table %s is supposed to be ignored but there is a foreign key "
"reference to it. "
"You may need to create this table manually at the dst schema before "
"running create-tables.",
table_name,
)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it the case that, even after refect(), metadata may still contain some of the ignored tables (if they are referenced by a foreign key) so we need to check and warn?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, exactly that

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was originally hoping that I wouldn't need to have the ignore option anywhere else other than here: You leave it out of the orm.py, and it's automatically not involved in ssg.py or anywhere else. But because of this thing, if there are FK references to the ignored table it'll still be in the orm.py and then you have to check for it in all the other stages, like create-tables and make-generators.

@mhauru mhauru merged commit e75a2db into main Sep 19, 2023
@Iain-S Iain-S deleted the ignorable-tables branch September 19, 2023 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add option to ignore tables completely
2 participants