ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Anton Vinogradov ...@apache.org>
Subject Re: Review needed for IGNITE-11410 Sandbox for user-defined code
Date Mon, 14 Oct 2019 06:09:34 GMT
Folks,

We should avoid heavy merges if possible.
I'm ok with IEP to keep tasks properly, but strictly against all-in-one
"+27000,-18200" merges.
This task implements Sandbox (API + core) which covered by tests and by
some integrations with existing components, which is enough to merge.
The most important thing here is that we will be able to speed-up Sandbox
coverage development once its core menged to the master.

On Fri, Oct 11, 2019 at 5:41 PM Вячеслав Коптилин <slava.koptilin@gmail.com>
wrote:

> Hi Denis,
>
> In my humble opinion, the security (the sandbox feature is about security,
> right?) either covers all APIs/subsystems or not.
> Security should work always and everywhere otherwise it is not security :)
>
> > From my point, we should divide the sandbox and features that use it.
> > Also, I added in the main features of Ignite (cache and compute) the
> sandbox calls.
> And at this point, you mixed both in the same pull-request.
>
> > I don't see any problem to have the sandbox in the master branch and
> implement covering for existing and new features if needed.
> On the other hand, this approach leads to ...
> ignite-123 [IEP-X] introduces new cool API
> ignite-124 [IEP-X] improved cool API
> ignite-125 [IEP-X] fixed a bug
> ignite-126 [IEP-X] fixed performance drop
> ignite-127 [IEP-X] Cache API uses new API
> ignite-127 [IEP-X] Compute grid uses new API
> ...
>
> Why should it be a part of the master branch history? All these things can
> be done on the feature branch, I think. Anyway, it is up to you.
>
> Thanks,
> S.
>
> пт, 11 окт. 2019 г. в 16:31, Denis Garus <garus.d.g@gmail.com>:
>
> > From my point, we should divide the sandbox and features that use it.
> > The sandbox is fully implemented and has needed tests.
> >
> > Also, I added in the main features of Ignite (cache and compute) the
> > sandbox calls.
> >
> > I don't see any problem to have the sandbox in the master branch
> > and implement covering for existing and new features if needed.
> >
> > пт, 11 окт. 2019 г. в 15:21, Вячеслав Коптилин <slava.koptilin@gmail.com
> >:
> >
> > > Hi Denis,
> > >
> > > Yep, I understand the scope of the ticket, but... I think it is not a
> > good
> > > idea to merge partly implemented feature(s) into the master branch.
> > > Especially, at this moment. We are at the stage of preparing a new
> > release
> > > and I doubt that all improvements, tests (unit tests, integration
> tests,
> > > and performance tests) can be implemented before the release branch is
> > cut
> > > off.
> > > Personally, I would prefer to create an epic/feature branch for these
> > > activities. In that case, we can implement a feature step by step and
> > merge
> > > it into the master branch once all components are covered.
> > >
> > > > But, sure, we should execute any user-defined code in the sandbox on
> a
> > > remote node. Feel free to create issues.
> > > will do.
> > >
> > > Thanks,
> > > S.
> > >
> > > пт, 11 окт. 2019 г. в 14:52, Denis Garus <garus.d.g@gmail.com>:
> > >
> > > > Hello, Slava!
> > > >
> > > > The scope of the issue is limited by the following features:
> > > >
> > > >    - StreamReceiver for DataStreamer;
> > > >    - EntryProcessor;
> > > >    - ComputeJob;
> > > >    - filter and transformer for ScanQuery.
> > > >
> > > > But, sure, we should execute any user-defined code in the sandbox on
> a
> > > > remote node.
> > > > Feel free to create issues.
> > > >
> > > > Thanks for the feedback!
> > > >
> > > > пт, 11 окт. 2019 г. в 13:26, Вячеслав Коптилин
<
> > slava.koptilin@gmail.com
> > > >:
> > > >
> > > > > Hello Denis, Anton,
> > > > >
> > > > > Could you please clarify the following aspect? Do we need the same
> > > > > changes/capabilities related to Continuous Queries, Disco
> listeners,
> > > > > CacheStore Factories etc?
> > > > >
> > > > > Thanks,
> > > > > S.
> > > > >
> > > > > пт, 11 окт. 2019 г. в 12:24, Anton Vinogradov <av@apache.org>:
> > > > >
> > > > > > Folks,
> > > > > >
> > > > > > As a prereviewer, I'd like to say that the solution looks good
to
> > me,
> > > > but
> > > > > > fresh eyes would be good.
> > > > > >
> > > > > > On Fri, Oct 11, 2019 at 9:40 AM Denis Garus <garus.d.g@gmail.com
> >
> > > > wrote:
> > > > > >
> > > > > > > Hello, Igniters!
> > > > > > >
> > > > > > > I've raised the PR [1] with the sandbox for AI [2].
> > > > > > > Could somebody review it?
> > > > > > >
> > > > > > > If you have questions and prefer the Slack, I've created
the
> > > channel
> > > > > [3].
> > > > > > >
> > > > > > > 1. https://github.com/apache/ignite/pull/6707
> > > > > > > 2. https://issues.apache.org/jira/browse/IGNITE-11410
> > > > > > > 3. https://app.slack.com/client/T4S1WH2J3/CP8JER880
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message