Skip to content

api: extend connect with fetch_schema param #271

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

Merged

Conversation

GRISHNOV
Copy link
Contributor

Added support of the fetch_schema parameter, which allows to ignore schema changes on the server.

By default, it is used fetch_schema = True:

>>> import tarantool
>>> conn = tarantool.Connection(host='localhost', port=3303)

>>> conn.schema_version
80

>>> conn.schema.schema
{257: <tarantool.schema.SchemaSpace object at 0x10fd21a80>, '_vinyl_deferred_delete': <tarantool.schema.SchemaSpace object at 0x10fd21a80>, 272: <tarantool.schema.SchemaSpace object at 0x10fd229b0>, ... ... , 'tester': <tarantool.schema.SchemaSpace object at 0x10fd22830>}

>>> tester = conn.schema.schema['tester']
>>> tester
<tarantool.schema.SchemaSpace object at 0x10fd22830>
>>> tester.
tester.arity    tester.flush()  tester.format   tester.indexes  tester.name     tester.schema   tester.sid      
>>> tester.format
{'id': {'name': 'id', 'type': 'unsigned', 'id': 0}, 0: {'name': 'id', 'type': 'unsigned', 'id': 0}, 'name': {'name': 'name', 'type': 'string', 'id': 1}, 1: {'name': 'name', 'type': 'string', 'id': 1}}

If the fetch_schema is specified as False, fields schema_version and schema will no longer be present in the connection object:

>>> import tarantool
>>> conn = tarantool.Connection(host='localhost', port=3303, fetch_schema=False)

>>> hasattr(conn, 'schema_version')
False
>>> hasattr(conn, 'schema')
False

In this case, requests to the server via the connection.update_schema will no longer be made when there is SchemaReloadException.

Closes #219

@GRISHNOV GRISHNOV force-pushed the igrishnov/gh-219-disable-schema-fetch-support branch from 88aaa84 to 0aeaef7 Compare December 12, 2022 15:19
@DifferentialOrange
Copy link
Member

In this case, requests to the server via the connection.update_schema will no longer be made when there is SchemaReloadException.

So there won't be any way to use a connection for any space operations? Only calls and evals? To be honest, I don't quite get this one without tests.

@GRISHNOV
Copy link
Contributor Author

In this case, requests to the server via the connection.update_schema will no longer be made when there is SchemaReloadException.

So there won't be any way to use a connection for any space operations? Only calls and evals? To be honest, I don't quite get this one without tests.

I suppose, it is.
By analogy with an existing solution:

c = netbox.connect(uri, {fetch_schema = false})
c.space -- always will be nil

@GRISHNOV GRISHNOV force-pushed the igrishnov/gh-219-disable-schema-fetch-support branch from 0aeaef7 to c462575 Compare December 19, 2022 19:24
@GRISHNOV
Copy link
Contributor Author

Now the connection with fetch_schema=False will throw an exception NotSupportedError when trying to call the methods replace, insert, delete, upsert, update, select.

@GRISHNOV GRISHNOV marked this pull request as ready for review December 22, 2022 10:29
Copy link
Member

@DifferentialOrange DifferentialOrange left a comment

Choose a reason for hiding this comment

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

Thank you for your PR and sorry I'm a bit late (I wasn't sure that it was ready for review).

@DifferentialOrange
Copy link
Member

You'll also need to rebase, sorry for the inconveniences

@GRISHNOV GRISHNOV force-pushed the igrishnov/gh-219-disable-schema-fetch-support branch 5 times, most recently from 244e67c to b4472cd Compare December 26, 2022 18:21
@GRISHNOV
Copy link
Contributor Author

While working on the task, I noticed that there is an issue in the CRUD tests.
When running make test locally, in the absence of an installed crud rocks, CRUD tests freezes forever.
So, I made some changes to the CRUD instance configuration file

Copy link
Member

@DifferentialOrange DifferentialOrange left a comment

Choose a reason for hiding this comment

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

Thank you for the updates! There are still some things we need to resolve

@GRISHNOV GRISHNOV force-pushed the igrishnov/gh-219-disable-schema-fetch-support branch 4 times, most recently from 639a9d5 to 3fc3701 Compare December 29, 2022 13:21
@GRISHNOV GRISHNOV force-pushed the igrishnov/gh-219-disable-schema-fetch-support branch 8 times, most recently from c57fdfb to eed5833 Compare January 9, 2023 14:23
@GRISHNOV GRISHNOV force-pushed the igrishnov/gh-219-disable-schema-fetch-support branch from eed5833 to b333b6f Compare January 9, 2023 14:42
@GRISHNOV GRISHNOV force-pushed the igrishnov/gh-219-disable-schema-fetch-support branch from b333b6f to fdc10f3 Compare January 10, 2023 20:36
Copy link
Member

@DifferentialOrange DifferentialOrange left a comment

Choose a reason for hiding this comment

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

Thank you for the updates! Let's fix remaining minor things and merge this one

@GRISHNOV GRISHNOV force-pushed the igrishnov/gh-219-disable-schema-fetch-support branch from fdc10f3 to 84b7617 Compare January 11, 2023 08:46
@GRISHNOV
Copy link
Contributor Author

Thanks for the feedback!

I've made all the corrections. At the moment there is a problem with tests that fall with variable probability. At the moment, it is difficult to determine the cause, but the problem, as far as I can see, is related to the update_schema call for connection pool:

# Turning the same connection into schemafull.
if mode is not None:
    for addr in con.pool.keys():
        con.pool[addr].conn.update_schema(con.pool[addr].conn.schema_version) # <------
    else:
        con.update_schema(con.schema_version)

It looks like there is a desynchronization with the transmitted con.pool[addr].conn.schema_version value and the value on the server

@GRISHNOV GRISHNOV force-pushed the igrishnov/gh-219-disable-schema-fetch-support branch from 84b7617 to 161a9c8 Compare January 11, 2023 13:40
Copy link
Member

@DifferentialOrange DifferentialOrange left a comment

Choose a reason for hiding this comment

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

Everything seems good, I'll wait till you resolve this flaky test issue.

@GRISHNOV GRISHNOV force-pushed the igrishnov/gh-219-disable-schema-fetch-support branch 4 times, most recently from f7edf6a to 15a5655 Compare January 12, 2023 09:10
In crud tests, if the necessary rocks dependencies are missing,
an error message is now displayed. Prior to this fix,
an error in importing missing models caused the tests to freez forever.

Part of #205
Added support of the fetch_schema parameter,
which allows to ignore schema changes on the server.

Closes #219
@GRISHNOV GRISHNOV force-pushed the igrishnov/gh-219-disable-schema-fetch-support branch from 15a5655 to d89651e Compare January 12, 2023 09:25
@DifferentialOrange
Copy link
Member

Ping me when everything is ready

@DifferentialOrange DifferentialOrange merged commit ca0a514 into master Jan 12, 2023
@DifferentialOrange DifferentialOrange deleted the igrishnov/gh-219-disable-schema-fetch-support branch January 12, 2023 11:08
DifferentialOrange added a commit that referenced this pull request Feb 13, 2023
DifferentialOrange added a commit that referenced this pull request Feb 13, 2023
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 this pull request may close these issues.

Allow to disable schema fetch
2 participants