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

database adapters: Missing migration + Wrong documentation + Access to instance complex #1396

Open
sveneberth opened this issue Jan 29, 2025 · 0 comments
Labels
annoying breaking-changes This PR causes breaking changes bug(fix) Something isn't working or address a specific issue or vulnerability Priority: High After critical issues are fixed, these should be dealt with before any further issues.

Comments

@sveneberth
Copy link
Member

sveneberth commented Jan 29, 2025

1. Missing Migrations

  • CustomDatabaseAdapter --> DatabaseAdapter
  • Skeleton.customDatabaseAdapter = X --> Skeleton.database_adapters = [X]
  • DatabaseAdapter's methods itself

2. Incorrect documentation

Furthermore the docstrings are wrong, they describe a not existing action parameter, but not is_add.


3. Missing deprecation hook

[ERROR] type object 'MySkel' has no attribute 'customDatabaseAdapter'

While there's a backward-compatibility on the SkeletonInstance, there's is no backward compatibility on Skeleton cls.


4. How to access a specific DatabaseAdapter instance?

In v3.6 an earlier we had one customDatabaseAdapter, so you could access it and its attributes directly:

table = my_skel.customDatabaseAdapter.table

Not it's wrapped by a list.
Either I memorize the index and make sure that nobody changes the order:

table = my_skel.database_adapters[1].table

or iterate and compare the type

table = None
for da in my_skel.database_adapters:
    if isinstance(da, SqlAdapter):
        table = da.table

Both ways are super unwieldy.


5. How should the the DatabaseAdapter know in which skeleton he is located?

In v3.6 an earlier I could implement __set_name__ and store the Skeleton cls in an attribute of my adapter and I knew in every method in the adapter to which Skeleton cls it belonged:

    def __set_name__(self, owner, name):
        self.skel_cls = owner

Now this is no longer possible, since __set_name__ is called on the list, but not on the adapter itself

@sveneberth sveneberth added annoying breaking-changes This PR causes breaking changes bug(fix) Something isn't working or address a specific issue or vulnerability Priority: High After critical issues are fixed, these should be dealt with before any further issues. labels Jan 29, 2025
@sveneberth sveneberth added this to the ViUR-core v3.7 milestone Jan 29, 2025
@sveneberth sveneberth changed the title Missing migration for database adapters database adapters: Missing migration + Wrong documentation Jan 29, 2025
@sveneberth sveneberth changed the title database adapters: Missing migration + Wrong documentation database adapters: Missing migration + Wrong documentation + Access to instance complex Jan 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
annoying breaking-changes This PR causes breaking changes bug(fix) Something isn't working or address a specific issue or vulnerability Priority: High After critical issues are fixed, these should be dealt with before any further issues.
Projects
None yet
Development

No branches or pull requests

1 participant