Dependencies: Migrate from crate[sqlalchemy] to sqlalchemy-cratedb

[amotl]

About

The CrateDB SQLAlchemy dialect needs more love, so it was separated from the DBAPI HTTP driver. This patch intends to accompany the migration.

Details

The new canonical package for the SQLAlchemy CrateDB dialect on PyPI is:

https://pypi.org/project/sqlalchemy-cratedb/

/cc @hammerhead, @hlcianfagna, @surister

[mindsdbadmin]

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

[amotl]

I have read the CLA Document and I hereby sign the CLA

[amotl]

recheck

[MinuraPunchihewa]

Hey @amotl,
Since we are also using the Create DBAPI, we will need to include it in the requirements.txt file. We can, of course, omit the sqlalchemy extra now.

[amotl]

Hi Minura,

thank you very much for looking into this.

The crate package is a dependency of sqlalchemy-cratedb, so you can omit it from the dependencies, and still import it: sqlalchemy-cratedb will take care of pulling in the right version. Currently, that is crate==1.0.0.dev0, but that will converge to 1.0.0 on the next iteration.

In general, crate is pretty stable, and will not change much on future iterations, because it implements the fundamental Python DB API specification pretty much completely already. In contrast to that, the sqlalchemy-cratedb package will receive a few more changes, and thus will observe a higher rate of upcoming releases. That is exactly the reason why we split both apart, see also apache/superset#29243 (comment).

Please let us know if that answers any questions well, also sharing more insights why we are observing this change at all.

With kind regards,
Andreas.

[MinuraPunchihewa]

Ah, I see. Thank you for letting us know, @amotl. The only issue is that we have a CI check that runs to ensure that the third-party packages used in our integrations are also listed as dependencies in the respective requirements.txt files. To avoid this failing, it would be great if you could list it here unless you think it will cause any sort of harm.

[amotl]

Hi. I've updated the patch like you suggested. If the build will pick up crate==1.0.0.dev0, because sqlalchemy-cratedb says so, it will be good to go.

Otherwise, please let me know if you see any other problems on CI. Thanks!

[MinuraPunchihewa]

Thank you, @amotl! Sorry for the trouble, but could you please sync your fork with the main branch? There is another CI check that is failing. I believe this is an issue on our end, but I remember that it was fixed frequently. I think syncing your branch with our main should do the trick.

[amotl]

Hi Minura,

I did a rebase yesterday whle already adjusting the patch according to your suggestions. Right now, I've used the "Rebase branch" button on GitHub once more.

With kind regards,
Andreas.

[MinuraPunchihewa]

Hey @hamishfagg,
Do you have any idea why this is still failing? I believe we added a check to ensure that this does not forked repos, if I am not mistaken?

[amotl]

I also think it must be something in mindsdb/github-actions, that trips because the PR is coming from the fork of an external contributor.

Feel free to use this as a blueprint example to eventually improve your shared actions on this detail of being more welcoming to external contributors.

If you see some room for improvements, please just go ahead and refresh (rebase) this page at your disposal, in order to trigger CI.

[hamishfagg]

Hey guys,
So we have been trying to think of a nice way to allow actions to run on forks for a while, but it's very tricky security-wise.
I've just made a change that will let things pass now, so long as we don't need to deploy it to a dev environment to test.
Luckily this is a simple PR so I think we can get away with not doing that?

Anyway this has prodded me to try get this sorted again, sorry for the inconvenience.

[MinuraPunchihewa]

Thanks, @hamishfagg. Yes, it won't be necessary to deploy it to a test environment right now. I will merge the PR.