Skip to content

test: fix execution with Tarantool 1.6 #198

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

Closed
wants to merge 4 commits into from

Conversation

oleg-jukovec
Copy link
Collaborator

@oleg-jukovec oleg-jukovec commented Jul 25, 2022

We declare support of Tarantool 1.6 in README.md:

go-tarantool/README.md

Lines 14 to 15 in 4b066ae

The package `go-tarantool` contains everything you need to connect to
[Tarantool 1.6+][tarantool-site].

go-tarantool/README.md

Lines 34 to 35 in 4b066ae

We assume that you have Tarantool version 1.6+ and a modern Linux or BSD
operating system.

It would be great for the tests to work with Tarantool 1.6.

The patch fixes the tests and adds run with Tarantool 1.6.9 to GitHub Actions.

I didn't forget about (remove if it is not applicable):

@oleg-jukovec oleg-jukovec force-pushed the oleg-jukovec/ci-tnt-1.6 branch 2 times, most recently from 8590796 to 1119547 Compare July 25, 2022 10:07
@oleg-jukovec
Copy link
Collaborator Author

oleg-jukovec commented Jul 25, 2022

As an alternative way we can update the README.md and start to support Tarantool 1.10+

@oleg-jukovec oleg-jukovec marked this pull request as ready for review July 25, 2022 10:18
Despite the fact that queue requires Tarantool 1.7 [1] it uses features
from Tarantool 1.7.4. `box.info.ro` [2] as an example. The same for
connection_pool.

The patch disables tests execution for queue and connection_pool with
Tarantool < 1.7.4.

1. https://github.com/tarantool/queue/blob/master/rpm/tarantool-queue.spec#L10
2. tarantool/tarantool@56462bc
We still support Tarantool 1.6, so we need to use Call16 by default.
The patch fixes tests execution results with Tarantool 1.6.
Tarantool 1.6.9 cannot be installed from a ready package, so we need
to build it manually.
@oleg-jukovec oleg-jukovec force-pushed the oleg-jukovec/ci-tnt-1.6 branch from 1119547 to af36e9b Compare July 25, 2022 10:22
@@ -187,7 +187,7 @@ func (connMulti *ConnectionMulti) checker() {
continue
}
var resp [][]string
err := connMulti.Call17Typed(connMulti.opts.NodesGetFunctionName, []interface{}{}, &resp)
err := connMulti.Call16Typed(connMulti.opts.NodesGetFunctionName, []interface{}{}, &resp)
Copy link
Member

@Totktonada Totktonada Jul 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not a strict equivalent, see tarantool/tarantool-java#196.

var tuples3 [][]Tuple3
err = conn.Call17Typed("func_name", []interface{}{}, &tuples3)
var tuples3 []Tuple3
err = conn.Call16Typed("func_name", []interface{}{}, &tuples3)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I really don't like about CALL_16: it implicitly changes a shape of the result (and the rules are not very obvious). Personally I would avoid it as much as possible. Or at least call only those functions, which return exactly what we'll receive on the connector side: so it will not be wrapped into an array or unwrapped. Maybe skipping of some tests is not so evil as pollute them with CALL_16.

I don't insist, just share my feeling about this.

Comment on lines +72 to +83
- name: Build Tarantool ${{ matrix.tarantool }}
if: matrix.tarantool == '1.6.9' && steps.cache-tnt-build.outputs.cache-hit != 'true'
run: |
sudo apt-get -y install git build-essential cmake make zlib1g-dev \
libreadline-dev libncurses5-dev libssl-dev \
libunwind-dev libicu-dev python3 python3-yaml \
python3-six python3-gevent
cd ${GITHUB_WORKSPACE}/tarantool
mkdir build && cd build
cmake .. -DCMAKE_BUILD_TYPE=Release
make
make DESTDIR=${TNT_BUILD_INSTALL_PATH} install
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow. I would do the following instead:

  • Build the package once for Ubuntu Focal.
  • Agree with @LeonidVas and @artembo where to place such special packages.
  • Optionally add support for setup-tarantool.

I don't insist: if it works, it is likely okay. However we can do it a bit better (faster, more durable, less code).

@Totktonada
Copy link
Member

As an alternative way we can update the README.md and start to support Tarantool 1.10+

In practical terms, nothing prevent us from support only 1.10+. OTOH, 1.6.9 support don't add much problems. I'm okay with any solution here.

@NeraverinTarantool
Copy link

We should explicitly switch to support only 1.10+. If users are not ready to update Tarantool, let them take an older version of the connector.

@oleg-jukovec oleg-jukovec added the wontfix This will not be worked on label Sep 14, 2022
@oleg-jukovec
Copy link
Collaborator Author

It was decided to make another pull request with switch to support only 1.10+ (README.md, comments).

oleg-jukovec added a commit that referenced this pull request Dec 25, 2022
Technically, the connector can now work with Tarantool 1.6+. But we
decided to simplify our CI and do not declare support for older
versions [1].

1. #198
oleg-jukovec added a commit that referenced this pull request Jan 12, 2023
Technically, the connector can now work with Tarantool 1.6+. But we
decided to simplify our CI and do not declare support for older
versions [1].

1. #198
@oleg-jukovec oleg-jukovec deleted the oleg-jukovec/ci-tnt-1.6 branch August 3, 2023 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants