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

TST: OmniSciDB - Enable logical value tests #2084

Merged

Conversation

xmnlab
Copy link
Contributor

@xmnlab xmnlab commented Feb 19, 2020

Enable tests that use queries without table and another tests that are already working on OmniSciDB

  • it depends on OmniSciDB v5.2

@anmyachev
Copy link
Contributor

@xmnlab what do you think about changing omnisci image in docker-compose.yml to omnisci/core-os-cpu-dev:latest (we already have several PRs that are tied to the latest changes from the omnisci side). By doing this, we can more actively add new features.

@xmnlab
Copy link
Contributor Author

xmnlab commented Feb 20, 2020

@anmyachev from a dev's perspective I think it is a good idea .. but from a user's perspective not sure if it could have side effects.

@saulshanabrook
Copy link
Contributor

@xmnlab and I just chatted about using the latest in the docker compose and I think it does make sense because there aren't currently (AFAIK) a large number of downstream ibis users who are need support for older omnisci versions. I think once that does become the case, we should make sure to document which versions of the server we support and test on those.

@xmnlab
Copy link
Contributor Author

xmnlab commented Feb 20, 2020

@jreback what are your thoughts about this?

@jreback
Copy link
Contributor

jreback commented Feb 20, 2020

this is probably fine
just document what the min versions are

@xmnlab
Copy link
Contributor Author

xmnlab commented Feb 20, 2020

sounds good. thanks!

@xmnlab xmnlab force-pushed the omniscidb-enable-logical-values-test branch from ad3ccef to 85ef796 Compare February 20, 2020 21:22
@xmnlab xmnlab marked this pull request as ready for review February 20, 2020 23:57
@xmnlab
Copy link
Contributor Author

xmnlab commented Feb 20, 2020

it is done for review! thanks!

@anmyachev
Copy link
Contributor

LGTM. @jreback what do you think about?

@xmnlab
Copy link
Contributor Author

xmnlab commented Feb 27, 2020

@jreback a gentle remind about this PR :)

@xmnlab
Copy link
Contributor Author

xmnlab commented Mar 2, 2020

depends on #2104

@xmnlab xmnlab added omnisci tests Issues or PRs related to tests labels Mar 6, 2020
@xmnlab xmnlab force-pushed the omniscidb-enable-logical-values-test branch from e1df947 to 858f5a1 Compare March 10, 2020 14:25
README.md Show resolved Hide resolved
@xmnlab xmnlab force-pushed the omniscidb-enable-logical-values-test branch from 858f5a1 to 1a3b45a Compare March 12, 2020 14:56
@xmnlab
Copy link
Contributor Author

xmnlab commented Mar 12, 2020

suggestions applied, ci green

README.md Outdated Show resolved Hide resolved
@xmnlab xmnlab force-pushed the omniscidb-enable-logical-values-test branch from a03db28 to eee0f65 Compare March 14, 2020 23:19
@xmnlab
Copy link
Contributor Author

xmnlab commented Mar 15, 2020

thanks for the review @jreback ! ci is green :)

@jreback jreback added this to the Next Feature Release milestone Mar 17, 2020
@jreback jreback merged commit 8ee7358 into ibis-project:master Mar 18, 2020
@jreback
Copy link
Contributor

jreback commented Mar 18, 2020

thanks

@xmnlab xmnlab deleted the omniscidb-enable-logical-values-test branch December 21, 2020 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Issues or PRs related to tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants