From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> To: Kirill Shcherbatov <kshcherbatov@tarantool.org>, tarantool-patches@freelists.org, georgy@tarantool.org Cc: alexander.turenko@tarantool.org Subject: [tarantool-patches] Re: [PATCH v3 1/6] box: rfc for stacked diagnostic area in Tarantool Date: Mon, 9 Sep 2019 21:44:17 +0200 [thread overview] Message-ID: <82acb857-925d-fd47-2cae-3c8592c7eb26@tarantool.org> (raw) In-Reply-To: <7b5287fa-c430-ca96-3b5b-0b879f28fba9@tarantool.org> > 2. >>> +``` >>> + >>> +Error cdata object has `:unpack()`, `:raise()`, `:match(...)`, >>> `:__serialize()` methods and uniform `.type`, `.message` and `.trace` >>> fields. + >>> + >> What if we want to throw a table error - as vshard does. > > https://github.com/tarantool/vshard/blob/master/vshard/error.lua > As fores I've understood, vshard has an own error representation same as box_error > adapter that constructs vshard-compatible table error object. VShard repeats error:unpack() format. If you change result of unpack(), then I will update VShard (if that will be necessary). > The backward compatibility wouldn't be broken with introducing a couple new fields in core error object. Yes, if you only add new fields to error:unpack(), then nothing will break. > However, to have a profit with new stacked diagnostics functionality the box_error adapter should be > updated correspondingly. This likely requires extending vshard error functionality. > I'd better create a related ticket for vshard module, if you are agree with me. > > > 3. >> I don't see any valid purpose for svp* methods. Please explain the cases when >> we need to rollback a diag area. Me neither. Svp won't solve any problems IMO. It will just complicate the code. Talking of cases below - if you call a function, you need to know how it works. If you called a function not setting a diag, and you expected a diag, that you either didn't test your code properly, or you was inattentive, or both. Svp won't solve neither of that problems. You still may forget to reset svp, to set svp, to test the code. > Kostya had a strong opinion about an ability to reset a last-errors set. Honestly, I didn't understand his point. And I think, if we ever will understand, we can easily add svps. But if we add them now and they appear to be useless, then we won't be able to drop them, because of compatibility. > 5. >>> +-- Return a reason error object for given error >>> +-- (when exists, nil otherwise) >>> +box.error.prev(error) == error.prev >>> +``` >> You let out of scope standard lua "error()" call. Please take it into >> consideration. > > I can't say nothing about error() call. It is not box error, it can't be extended. > It should be deprecated when #4398 would be closed, right? error() is a built-in Lua function, it is a part of the language. You can't deprecate it. But I also don't understand what is wrong with it. Seems like it will work just like always.
next prev parent reply other threads:[~2019-09-09 19:40 UTC|newest] Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-09-02 14:51 [tarantool-patches] [PATCH v3 0/6] box: stacked diagnostics area in fiber Kirill Shcherbatov 2019-09-02 14:51 ` [tarantool-patches] [PATCH v3 1/6] box: rfc for stacked diagnostic area in Tarantool Kirill Shcherbatov 2019-09-09 8:13 ` [tarantool-patches] " Kirill Shcherbatov 2019-09-09 19:44 ` Vladislav Shpilevoy [this message] 2019-09-09 19:44 ` Vladislav Shpilevoy 2019-09-02 14:51 ` [tarantool-patches] [PATCH v3 2/6] box: rename diag_add_error to diag_set_error Kirill Shcherbatov 2019-09-02 14:51 ` [tarantool-patches] [PATCH v3 3/6] box: introduce stacked diagnostic area Kirill Shcherbatov 2019-09-02 14:51 ` [tarantool-patches] [PATCH v3 4/6] box: extend box.error.new({}) API Kirill Shcherbatov 2019-09-02 14:51 ` [tarantool-patches] [PATCH v3 5/6] iproto: refactor error encoding with mpstream Kirill Shcherbatov 2019-09-04 10:53 ` [tarantool-patches] " Konstantin Osipov 2019-09-02 14:51 ` [tarantool-patches] [PATCH v3 6/6] iproto: support stacked diagnostics for errors Kirill Shcherbatov
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=82acb857-925d-fd47-2cae-3c8592c7eb26@tarantool.org \ --to=v.shpilevoy@tarantool.org \ --cc=alexander.turenko@tarantool.org \ --cc=georgy@tarantool.org \ --cc=kshcherbatov@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='[tarantool-patches] Re: [PATCH v3 1/6] box: rfc for stacked diagnostic area in Tarantool' \ /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