Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Yan Shtunder <ya.shtunder@gmail.com>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH v4 net.box] Add predefined system events for pub/sub
Date: Tue, 1 Mar 2022 23:44:02 +0100	[thread overview]
Message-ID: <493ecf9a-eeb8-a928-d358-947f05a5c32d@tarantool.org> (raw)
In-Reply-To: <20220228133255.62874-1-ya.shtunder@gmail.com>

Hi! Thanks for the fixes!

In the design document it was also said:

	The events must be available from the very beginning as not MP_NIL.
	It will come in handy when local subscriptions will be supported (not
	via the network). Otherwise no way to detect whether an event is even
	supported at all by this Tarantool version. By design absence of a
	subscription key equals MP_NIL and no events are delivered ever except
	this MP_NIL itself. It happens both if it is not broadcast and if it
	simply does not exist in the code.

And

	The builtin events should not allow userspace broadcast. Meaning, users
	should not be able to call box.event.broadcast(‘box.status’, any_data).

In the current implementation the built-in events override is still
allowed, and before box.cfg{} is called, all the events will return nil.

For how to ban built-in events override - in the design doc there is a couple
of proposals. A third option would be to check for "box." prefix in the public
API of box.broadcast() and return an error if it matches. I would probably ask
Vladimir D. for what he thinks is the best among these options. I think he will
be the second reviewer anyway.

For what to broadcast before box.cfg{} - I have no strong opinion here.
Option 1 is to broadcast some invalid default values like `box.schema = {version = 0}`.
Option 2 would be to broadcast an empty dictionary. For instance, before box.cfg is
called, the following values are available:

	box.id = {}
	box.schema = {}
	box.status = {}
	box.election = {}

This way the users will be able to distinguish an event being not supported
at all from box.cfg{} being not called yet. Otherwise they would need to parse
_TARANTOOL version string locally and peer_version in netbox.

The rest looks good!

  reply	other threads:[~2022-03-01 22:44 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-28 13:32 Yan Shtunder via Tarantool-patches
2022-03-01 22:44 ` Vladislav Shpilevoy via Tarantool-patches [this message]
  -- strict thread matches above, loose matches on Subject: below --
2022-03-24 14:01 Yan Shtunder via Tarantool-patches
2022-03-25 23:44 ` Vladislav Shpilevoy via Tarantool-patches
2022-03-22 11:51 Yan Shtunder via Tarantool-patches
2022-03-22 22:55 ` Vladislav Shpilevoy via Tarantool-patches
2022-02-22 12:56 Yan Shtunder via Tarantool-patches
2022-02-23 22:44 ` Vladislav Shpilevoy via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=493ecf9a-eeb8-a928-d358-947f05a5c32d@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    --cc=ya.shtunder@gmail.com \
    --subject='Re: [Tarantool-patches] [PATCH v4 net.box] Add predefined system events for pub/sub' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox