Meteor-Community-Packages / organization

Discussions on organization of the organization 🎩
https://meteorjs.community/
41 stars 1 forks source link

New package for inclusion - reywood:publish-composite #10

Closed filipenevola closed 5 years ago

filipenevola commented 5 years ago

Package/project name & description

reywood:publish-composite https://github.com/englue/meteor-publish-composite

Links

Current status of the project

I'm willing to maintain this package but it's probably a better idea to move it to here instead of keeping it under Pathable organization.

Is someone else using this package? Is there already an active fork?

cc @englue @reywood

SimonSimCity commented 5 years ago

I propose adding a πŸ‘reaction to the initial comment if you would like to see this package supported (e.g. because you use reywood:publish-composite or any of its forks).

Just FYI: Support doesn't necessarily mean moving it here though.

harryadel commented 5 years ago

It's surely a great package but there's an alternative which achieves the same thing in much better capacity.

And it's actively maintained too!

Disclaimer: I'm affiliated with the organization behind the package

StorytellerCZ commented 5 years ago

@harryadel I think Grapher is a bit of an overkill for someone who wants just composite publishes.

harryadel commented 5 years ago

You're 100% correct.

I merely suggested it as directing developers over to another, maintained, package is less work compared to actually attempting to maintain one. And, as I've read before in your discussions slimming down and focusing on critical matters (i.e. directly contributing to Meteor core) is a lot more favored!

This isn't an attempt to discourage any of your amazing efforts though, have a good one! :)

mitar commented 5 years ago

You can always use: https://github.com/peerlibrary/meteor-reactive-publish

filipenevola commented 5 years ago

@mitar I don't know how I didn't know about this package. Thank you, I'll analyze this as an alternative.

Do you know of any side effect in memory consumption in the server due to keeping these autorun contexts in the server?

mitar commented 5 years ago

Memory consumption not really. The the only issue is that because computation is a black box it might rerun (requery) more often than if you would manually optimize the join query and join conditions.

It does open observeChanges for every query, which does consume memory. But that probably all of those packages have to do.

SimonSimCity commented 5 years ago

@mitar correct me if I'm wrong, but don't I have to combine https://github.com/peerlibrary/meteor-reactive-publish and https://github.com/peerlibrary/meteor-reactive-mongo in order to get the same result as https://github.com/englue/meteor-publish-composite?

I have the impression that I'm missing on a separation on what which of these packages is responsible for and in which way it would prove to be the better alternative.

I'm currently using the package in request for almost every publication and it came very handy, but I wouldn't have much of a clue which package of the ones named above would fit my need.

Here's a simplified example of how I use it. How would you've written it instead?

Meteor.publishComposite('devices', (selectors, options) => ({
  find() {
    return Devices.find(selectors, options);
  },
  children: [
    {
      find(device) {
        return Meteor.users.find(
          { _id: `${device.last_used_by}` }
        );
      },
      children: [
        {
          find(user) {
            return Teams.find(
              { _id: `${user.teamId}` }
            );
          },
        },
      ],
    },
    {
      find(device) {
        return DeviceGroups.find(
          { _id: { $in: device.deviceGroupIds } }
        );
      },
    },
  ],
}));
mitar commented 5 years ago

Yes, you have to use reactive mongo if you want to use reactive publish on MongoDB queries. But you can also use some other reactive source with reactive publish (like reactive variable which changes every second). And also you can use reactive mongo inside server-side autorun outside of reactive publish context. This is why there are three packages.

SimonSimCity commented 5 years ago

@mitar could you please provide me an example of how to query my data-set in a performant way using your extension(s)?

mitar commented 5 years ago

Sorry, no time for that. But if it is a M2M relation then probably reactive publish is not a good choice.

filipenevola commented 5 years ago

@SimonSimCity I guess the main benefit of peerlibrary/meteor-reactive-publish is that you don't need to query many times. Using your example:

If you return 1000 devices then each children query below will be performed 1000 times, one each time, which can impact your db performance.

Meteor.publishComposite('devices', (selectors, options) => ({
  find() {
    return Devices.find(selectors, options);
  },
  children: [
    {
      find(device) {
        return Meteor.users.find(
          { _id: `${device.last_used_by}` }
        );
      },
      children: [
        {
          find(user) {
            return Teams.find(
              { _id: `${user.teamId}` }
            );
          },
        },
      ],
    },
    {
      find(device) {
        return DeviceGroups.find(
          { _id: { $in: device.deviceGroupIds } }
        );
      },
    },
  ],
}));

Now using an alternative solution with mentioned package (I never used it then maybe I miss understood something but I think I got the idea):

Meteor.publish('devices', function () {
  this.autorun(function (computation) {

      const devicesCursor = Devices.find(selectors, options);
      const lastUsersIds = devicesCursor.map(({last_used_by: lastUsedBy}) => lastUsedBy);
      const deviceGroupIds = devicesCursor.flatMap(({deviceGroupIds}) => deviceGroupIds);
      const usersCursor = Meteor.users.find(
              { _id: { $in: lastUsersIds } }
            );
      const deviceGroupsCursor = DeviceGroups.find(
              { _id: { $in: deviceGroupIds } }
            );
      const teamsIds = usersCursor.map(({teamId}) => teamId);
      const teamsCursor = Teams.find(
              { _id: { $in: teamsIds } }
            );

      return [devicesCursor, deviceGroupsCursor, usersCursor, teamsCursor];
  });
});

See now that the number of queries (different requests to the database) is constant, always 4, no matter how many devices the first query will return.

To be clear: 1 - I never used the meteor-reactive-publish because I just discovered it here in this thread but I guess that is the idea. 2 - I still think it's better to use publish-composite as first solution, then if the quantity of queries against your db is too high you can switch to meteor-reactive-publish but keep in mind that any change on any of these cursors are going to execute all the four queries again because the autorun block has no idea of which query exactly need to be refetched (but probably better than a first loading with 1000 x 4 queries) but of course depends on each use case.

I hope this helps.

TechplexEngineer commented 5 years ago

Are other people using aggregations and the new $lookup to push the work to the database?

I've been structuring my new code like this:

import { ReactiveAggregate } from 'meteor/tunguska:reactive-aggregate';

import Beacons from '/lib/collections/beacons.c.js';

Meteor.publish('beacons_table', function(tableName, ids, fields) {
    check(tableName, String);
    check(ids, Array);
    check(fields, Match.Optional(Object));

    ReactiveAggregate(this, Beacons, [
        // Table determines which rows to display in separate query
        {
            $match: {
                // Standard mongo query syntax
                _id: { $in: ids },
            },
        },
        {
            $lookup : {
                from         : 'Hubs',
                localField   : 'reportHubId',
                foreignField : 'deviceId',
                as           : 'hub',
            },
        },
        // $unwind used for getting data in object or for one record only
        { $unwind: '$hub' },
        // Only include requested fields
        { $project: fields },
    ]);
});

Note: this query is used by an instance of a tabular table

SimonSimCity commented 5 years ago

@TechplexEngineer it's an interesting plugin you're using here ... but https://github.com/robfallows/tunguska-reactive-aggregate/issues/9 is still keeping your example from working truly reactive.

pmogollons commented 5 years ago

@filipenevola thats correct, we used to use publishComposite on an old project and then we migrated the publications to reactive-publish, those were much more performant as it queries the DB less, but requires some additional work than publishComposite.

Now we use grapher, and I think I read it uses some fork of publishComposite, but im not sure right now.

StorytellerCZ commented 5 years ago

OK, let's get this moving.

We have reywood:publish-composite as the base.

Do we want just that or do we want to get some improved version?

mitar commented 5 years ago

So can that be moved or are we forking? (Has anyone tried to contact the maintainer over the e-mail or so?)

I think the best is to start with same version and then decide what to improve/merge other forks in.

copleykj commented 5 years ago

I think we should pursue moving rather than forking as the first path.

filipenevola commented 5 years ago

@mitar I tried to contact him via issues, no answer so far https://github.com/englue/meteor-publish-composite/issues/122.

For me both works, fork or move. I can create PRs to update the package later depending on the choice.

Thanks.

mitar commented 5 years ago

@filipenevola In the past I had good success writing to an e-mail address maintainer commits with, visible often with git log. In this case maintainer has their public e-mail address there, so you could try writing to there as well.

Also, I see that a new release was made 17 days ago. So it is still slightly maintained?

copleykj commented 5 years ago

@mitar I believe a merged PR introduced a major issue so they reverted and re-released

filipenevola commented 5 years ago

I think as he didn't respond an issue from May 13th would be fair to assume the repo is not maintained.

Then I could make a PR to merge my changes.

mitar commented 5 years ago

I mean, since your issue versions have been still made. So I cannot really agree that it is not maintained? Only that they are not responding to issues?

Have you tried sending them an e-mail?

filipenevola commented 5 years ago

Just sent him an email, let's see.

filipenevola commented 5 years ago

Hello, I have good news, Sean, the package author, replied my email and he agreed to move his repo under Meteor community organization, I asked him to reply here then we can help him moving.

reywood commented 5 years ago

Hi everyone. Shall I transfer the repo to Meteor-Community-Packages?

mitar commented 5 years ago

@reywood I just added you to the organization. Accept the invite. This should allow you to add transfer the repository to the organization.

Fen747 commented 5 years ago

IMHO, though publish-composite came in handy for me while prototyping one or twice, it's far too hard to truly optimize such nested reactivity, I've always preferred writingy own observers to drastically improve performances with often less lines of code.

reactive-publish is somewhere between the two if you want nested reactivity with little code but are not yet too demanding on server-side optimisations.

Finally, Grapher and Apollo Subscriptions are top high end production-grade scalable solutions.

To conclude, composite-publish imho, is far from being a prioritary package requiring maintainance, as I personally don't consider in it's essence as a production quality package that shouldn't be replaced after prototyping.

I've seen way too much Meteor developpers putting reactivity anywhere and everywhere when other solutions were as easy to write and far more efficient, often because of not really thinking about it or overestimating where reactivity is truly needed :)

filipenevola commented 5 years ago

Hi David, thanks for your considerations but I have strict conditions and I need to use reactivity everywhere in one of my Meteor apps and I use a combination of reactive-publish and publish-composite depending on the data structure involved then at least for me it's important as I use it every day.

Then I think is not a good direction to affirm that some packages are worth or not based on app choices and problems, if we have people willing to provide good maintenance level I think the package is worth, at least for a group of people.

See that we had 16 likes in the first message then at least somebody is still using this package.

We also need to consider the time to market, in this project specifically Apollo or Grapher was not available in a good shape when it was started (more than 3 years ago and I was not part of the team also to explain how was the decision process) then we would have to invest money to use new approaches like this in a massive codebase.

btw, in any new project, I'm always using GraphQL with Apollo πŸ˜‰

Fen747 commented 5 years ago

Hi Filipe !

Well, I didn't exactly means some are worth and others are not, it was just all expressing my point of view on what makes me think some packages are, or seems to be, at least, more urgent to maintain than others (many core packages are in need of updates imho). πŸ˜‰

That's not to say that nobody else that find interest in doing so should not maintain the package, of course not πŸ˜…

I'm very prone to considering time to market, as I myself have specialized for the last 5 years in accompanying or building startups (thank you Meteor ❀️ ), which is when packages like publish-composite or others had the possibility to come in very handy in the early phases, before I took the time to learn what seems to me as better cost and time effective alternatives πŸ˜„

I would also add that I'm one of those guys who's very concernened on reducing the carbon dioxyde (yes, we actually exists ahah) their code emits or the number of server required to handle a maximum of actions (even yesterday, I was at another Tech For Good event about how much unoptimized code bases can be very polluting), so please, excuse my biases, I didn't mean at all to be rude or deem reywood's work unworthy.

StorytellerCZ commented 5 years ago

I see that @reywood moved the repository. As such this is now call for maintainers. Write here if you are interested. So far I've added @reywood and @filipenevola . Not sure where everyone else stands.

filipenevola commented 5 years ago

Great. Do we want to keep the old package name or to rename it to have a common prefix with other community packages? IMO a common prefix is good.

copleykj commented 5 years ago

I don't think that moving packages under a common org will do anything except further confuse people, and I think that should be avoided any time it's possible.

Fen747 commented 5 years ago

A common prefix might be a good way to show an active maintaining of packages, with a common core team and some organization, as well as a indicator of code quality and methodology shared across them. And in addition, it could allow us to get rid of many back-compat problems as well.

filipenevola commented 5 years ago

I understand your point @copleykj but right now it's very hard to find the right version of a specific package that we want.

I think would be pretty useful to know that any package under mcp: or whatever: is safe and well maintained but maybe we should discuss that in a different issue.

mitar commented 5 years ago

Great. Do we want to keep the old package name or to rename it to have a common prefix with other community packages? IMO a common prefix is good.

I think I would reuse existing Meteor package name as much as possible (possible: existing maintaner is open to this and is active enough to add communitypackages as maintainer on the package). We have done this with other packages as well.

I think that moving the repository is a good enough signal that things are being maintained, and also of course that there will be less issues and more PRs merged. I think those last things is the core way to make something look maintained, and really be maintained (we do not want just to look, but really be).

BTW, all this is laid out in this section of README. I think if you would like to discuss general questions of how to add a new package, open an issue in organization repository, or open a PR with suggested changes. I do not think it is suitable to discuss this in this thread, where we have original maintainer present and bug them with more notifications.

mitar commented 5 years ago

@reywood if you could also add communitypackages to maintainers of the existing package name, that would allow us then to reuse the name:

meteor admin maintainers reywood:publish-composite --add communitypackages
reywood commented 5 years ago

done

StorytellerCZ commented 5 years ago

Alright closing this as all the main points have been satisfied.