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

!quit command should return success #425

Closed
julianhyde opened this issue Feb 22, 2021 · 6 comments
Closed

!quit command should return success #425

julianhyde opened this issue Feb 22, 2021 · 6 comments

Comments

@julianhyde
Copy link
Owner

julianhyde commented Feb 22, 2021

In Calcite CI, using the new SQLLine 1.10, a test that previously succeeded now fails. The command is ./sqlline -e '!quit'.

Here is the CI output (original here):

  ./sqlline -e '!quit'
  shell: /bin/bash -e {0}
  env:
    _JAVA_OPTIONS: -XX:GCTimeLimit=90 -XX:GCHeapFreeLimit=35
    JAVA_HOME_15.0.2_x64: /Users/runner/hostedtoolcache/jdk/15.0.2/x64
    JAVA_HOME: /Users/runner/hostedtoolcache/jdk/15.0.2/x64
    JAVA_HOME_15_0_2_X64: /Users/runner/hostedtoolcache/jdk/15.0.2/x64
Picked up _JAVA_OPTIONS: -XX:GCTimeLimit=90 -XX:GCHeapFreeLimit=35
Downloading https://services.gradle.org/distributions/gradle-6.8.1-bin.zip
..........10%..........20%..........30%...........40%..........50%..........60%..........70%...........80%..........90%..........100%
Building Apache Calcite 1.27.0-SNAPSHOT
Picked up _JAVA_OPTIONS: -XX:GCTimeLimit=90 -XX:GCHeapFreeLimit=35
Feb 22, 2021 7:58:45 AM org.jline.utils.Log logr
WARNING: Unable to create a system terminal, creating a dumb terminal (enable debug logging for more information)
sqlline version 1.10.0
Error: Process completed with exit code 2.

@snuyanzin, Do you think that behavior in a CI setting might have changed because we upgraded from JLine? Do you know of a way to silence that "Unable to create a system terminal" warning?

@snuyanzin
Copy link
Collaborator

It seems the main issue is not Unable to create a system terminal, creating a dumb terminal.
Usually it means that highlighting and some other terminal features are not supported however the main functionality still has to keep working.
For instance for the same action build for Windows here contains the same wording.
I suppose that the main problem here is Error: Process completed with exit code 2.
#409 which does not take into account !quit command

I tried a workaround for calcite like ./sqlline -e '!quit' || echo 'Work around' instead of ./sqlline -e '!quit'
and it was built ok here

Regarding Unable to create a system terminal may be it makes sense to force build of jna or jansi terminal to avoid this

@julianhyde
Copy link
Owner Author

julianhyde commented Feb 22, 2021

You're right, the problem is not the system terminal, it's the status code.

@julianhyde julianhyde reopened this Feb 22, 2021
@julianhyde julianhyde changed the title Unable to create a system terminal !quit command should return success Feb 22, 2021
@julianhyde
Copy link
Owner Author

We might need a new SQLLine release for this. I'll see if I have time to do one this week.

@snuyanzin
Copy link
Collaborator

There is still a workaround which I tried on my fork of calcite and which seems working
snuyanzin/calcite@18756e4
and github actions for that commit
https://github.com/snuyanzin/calcite/actions/runs/590457452

@julianhyde
Copy link
Owner Author

The purpose of that test is to make sure that SQLLine (and the shell scripts that launch it in a Calcite environment) is working. With your workaround, the test is useless.

@julianhyde
Copy link
Owner Author

Fixed in b20d3e0. Thanks for the quick fix, @snuyanzin!

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 a pull request may close this issue.

2 participants