Recommended approach for production discourse using PR (not merged)

Hello,

We are needing to use this new feature/functionality for email:

Since it is not merged (we assume it will be one day) what is the recommended way to run a production discourse and include a PR in review?

I assume we have to avoid/not update discourse for the regular updates, but thats’ probably over simplifying the approach.

Any guidance on how others work in this scenario would be appreciated.

  • Jake

First: I don’t know.

But I think this might work:

cd /var/discourse
./launcher enter app
cd /var/www/discourse
su - discourse -c 'git fetch origin pull/<pr_number>/head:<local_branch_name>'
su - discourse -c 'git switch <local_branch_name>'
sv restart unicorn

If that does work, then you could add stuff to your app.yml to make it do that during the build. Or maybe it’ll get merged soon and you can just wait it out.

If that makes things worse, you can do a

 ./launcher destroy app;./launcher start app

and that’ll put back the image that you last built.

3 Likes

That’s very helpful thank you. Ideally we’d like to wait till it’s merged up but being new to this it’s not clear if that’s a few days, weeks, or months.

Thank you again.

No criticism of this PR (I didn’t look at it in detail), but in general I can’t think this is good practice.

  • you will get no updates from core whilst you are stuck on this branch, that includes any security patches.
  • you may get some instability from changes in the branch (which has to be treated as development code until merged)
  • what do you do if it’s rejected?
  • heck the tests haven’t even run yet?

Take advantage of CDCK developer review and wait til it’s reviewed & merged?

4 Likes

That’s good advice.

With what I suggested, you might be able to see that it actually works (or maybe there are spec in there that answer that question), or get by for a while until it’s accepted. Lots of people wait weeks (or months) to upgrade amyway.

2 Likes

@merefield thanks - I believe you are saying just wait until it gets merged, is that correct?

We agree, that’s a great idea. In the meantime however we need to handle email bounces.

Again, we don’t know how long the process takes, so without that knowledge, we will assume it will take a good amount of time.

Maybe I’m missing some nuance here…

I think you’re safe to try it out for some weeks. If there is another release, you can decide whether to update your PR to work with the next release or find some other solution. Probably the easiest thing would be to do it in a plugin?

Wait. Why not just do it in a plugin?

That’s the usual course of action. Do it in a plugin and then ask if they’d be interested in a PR. Right now, it seems that you’re the only one on the planet who wants this. Adding it to core means that someone will have to maintain it indefinitely; it’s not trivial.

3 Likes

Yeah, I don’t understand why this isn’t implemented in a plugin.

Then you could simply evolve the plugin separately to the core install.

Once (if) the PR is merged, you could then remove the plugin!

1 Like

We also need to solve this problem for non-public security patches.

We have code in our internal tooling that merges in a branch from a repository - I’d recommend the same approach for you.

Something like this should work for you in an exec block (probably in the after_code hook):

    # fetch and merge the patch
    git merge REFERENCE --no-commit
    bundle install # if necessary
    pnpm install --frozen-lockfile # if necessary
5 Likes

@merefield @pfaffman it’s not a plugin simply because, for us, that’s not trivial. We have never written a plugin. It someone has some directions on how to wire this up, we’d be happy to look at it!

Also, I would probably not say we are only person that ‘wants’ netcore - it’s one of the largest ESP…on earth, and many times bigger than some of the others supported in core. I’m not suggesting it’s better, or users may want the others, but netcore is a very large, well regarded ESP. In fact you can see a bunch of talk about it here, as it was formerly pepipost:

https://meta.discourse.org/search?q=pepipost

2 Likes

A dual strategy would be appropriate I feel:

  • Build this as a plugin now, go live
  • Negotiate/discuss PR with CDCK

Not being able to build a plugin is not great reason to PR.

Third parties could still use your plugin.

5 Likes

@merefield sorry why do you think the plugin build is related to the PR creation? Netcore themselves wrote the PR.

Maybe I’m missing some detail in what you’re saying.

1 Like

Just a suggestion. It gives you added flexibility on deployment timelines. Who doesn’t like fewer dependencies?

2 Likes

Because you can create a plugin.

You do not know if they will ever accept your PR. And neither do I.

Here’s a hint: Someone from Team has responded in this topic and they didn’t say “Yeah, we’re pulling this in ASAP”. They instead gave you “Here’s what we do if we have a PR that won’t be accepted into core for months”.

It would appear that those are your choices.

I wouldn’t read this much into my response.

I’m on the infra side, I have no insight into the dev teams priorities. To me the commit looks :+1:t3: but a more practiced eye might have a different opinion.

But I do think this answering this question would be a generally useful piece of advice / FAQ for self-hosters.

IMO a plugin would be too heavy here.

1 Like

Well, shows what I know.

And I keep forgetting just how big the staff is now and how segmented the teams must be. It seems like just yesterday that most everyone knew most everything (of course even then people had their niches), but that “yesterday” was eight years ago.

3 Likes

(post deleted by author)

1 Like