MongoEngine / flask-mongoengine

MongoEngine flask extension with WTF model forms support
Other
842 stars 254 forks source link

Compatibility issue with mongoengine 0.24.1 #451

Closed robertzak closed 2 years ago

robertzak commented 2 years ago

In 0.24.1 mongoengine 'added a check for uuidRepresentation in the get_connection_settings function.

Unfortunately flask-mongoengine is doing a lower() on all connections parameters in the _sanitize_settings function.

This doesn't appear to have caused me issues with other camel case parameters (ex: tlsCertificateKeyFile, tlsCAFile) etc, so I'm not sure if pymongo itself is not case sensitive.

I don't know if this is as simple as removing the lower() in the sanitize. I assume its an issue if you tried to do something like as well:

MONGO_DB_UUIDREPSENENTATION="standard"

instead of:

MONGODB_SETTINGS = {
         "uuidRepresentation": "standard"
}

Version info:

Python 3.10.4 Flask 2.1.2 flask-mongoengine 1.0.0 mongoengine 0.24.1 pymongo 4.1.1

gmclark commented 2 years ago

same issue here. is anyone still maintaining this repo @wojcikstefan?

gmclark commented 2 years ago

perhaps the solution would be something like:

for k, v in settings.items():
        if k.startswith("MONGODB_"):
            k = k[len("MONGODB_") :].lower()
        resolved_settings[k] = v

that way, separate MONGODB_DB, MONGODB_HOST, etc are lowercased, but ones included directly in MONGODB_SETTINGS are left alone

insspb commented 2 years ago

@gmclark I have some time to maintain this repo right now. But it will take some time to back to this code, as I was not here for two years and completely not working with Flask/Mongo anymore.

As far as I remember this issue is fixed in #429 work, but not released to master/pip yet. I will recheck it before 2.0.0 release, as it seems very important.

Thank you for patience.

insspb commented 2 years ago

@gmclark @robertzak I added fix for this and all possible cases to current work in #429

But actually it is mongoengine error that they expect concrete setting name, as pymongo itself does not care.

robertzak commented 2 years ago

Yeah, I realized after I made this issue that the real problem was in mongoengine. I made an issue with https://github.com/MongoEngine/mongoengine/issues/2650, and even offered to submit a PR, but I haven't had any response. I think the maintainers are no longer active or something.

Thanks for fixing things here at least.

gmclark commented 2 years ago

Thanks very much for the update @insspb. Do you have any estimate on when there will be a release with these changes?

insspb commented 2 years ago

@gmclark It available in master, already.

I need some inspiration to mass update documentation for all last changes. And have some work to do on main work... So release date to pip may vary from few days to 1-2 months, as current release will bring changes that was accumulated for 2 years.

for me it is very hard to maintain this project at this time, that's why I was not here so long. I do not have any production flask or mongo, so not always in 'context' of latest trends/changes etc. That's why I significantly extended CI/CD tests matrix. To at least catch any compatibility issues earlier.