Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add support to use TRANSPORTER env var for monolith deployments #29373

Merged

Conversation

sampaiodiego
Copy link
Member

Proposed changes (including videos or screenshots)

Issue(s)

Steps to test or reproduce

Further comments

@changeset-bot
Copy link

changeset-bot bot commented May 25, 2023

🦋 Changeset detected

Latest commit: f4e83c8

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 28 packages
Name Type
@rocket.chat/meteor Minor
@rocket.chat/core-typings Minor
@rocket.chat/rest-typings Minor
@rocket.chat/api-client Patch
@rocket.chat/core-services Patch
@rocket.chat/cron Patch
@rocket.chat/gazzodown Major
@rocket.chat/model-typings Patch
@rocket.chat/ui-contexts Major
@rocket.chat/account-service Patch
@rocket.chat/authorization-service Patch
@rocket.chat/ddp-streamer Patch
@rocket.chat/omnichannel-transcript Patch
@rocket.chat/presence-service Patch
@rocket.chat/queue-worker Patch
@rocket.chat/stream-hub-service Patch
@rocket.chat/omnichannel-services Patch
@rocket.chat/pdf-worker Patch
@rocket.chat/presence Patch
rocketchat-services Patch
@rocket.chat/ddp-client Patch
@rocket.chat/fuselage-ui-kit Major
@rocket.chat/models Patch
@rocket.chat/ui-client Major
@rocket.chat/ui-video-conf Major
@rocket.chat/uikit-playground Patch
@rocket.chat/web-ui-registration Major
@rocket.chat/instance-status Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@codecov
Copy link

codecov bot commented May 25, 2023

Codecov Report

Merging #29373 (f4e83c8) into develop (d19fed6) will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop   #29373   +/-   ##
========================================
  Coverage    46.66%   46.66%           
========================================
  Files          698      698           
  Lines        13071    13071           
  Branches      2226     2226           
========================================
+ Hits          6099     6100    +1     
- Misses        6652     6653    +1     
+ Partials       320      318    -2     
Flag Coverage Δ
e2e 46.62% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

rodrigok
rodrigok previously approved these changes May 26, 2023
To use that, you can use the `TRANSPORTER` env var adding "monolith+" to the transporter value. To use NATS for example, your env var should be:

```bash
export TRANSPORTER="monolith+nats://localhost:4222"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a real downside to just making it use transporter just like normal? This means if someone deploys this then wants to move to microservices they’ve got to now remove that instead of add microservices

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah I agree with Aaron

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the problem is the current TRANSPORTER env var is used to turn micro services on.. that's why I came up with a different "protocol"..

export function isRunningMs(): boolean {
return !!process.env.TRANSPORTER?.match(/^(?:nats|TCP)/);
}

the initial idea was to use a completely different env var.. I even used MONOLITH_TRANSPORTER initially but then changed to current approach..

I'm open to change it though.. but if want to use same env var with same value we'd need a new way to trigger other micro services specific code.

@debdutdeb
Copy link
Member

debdutdeb commented May 29, 2023

this I believe will break at addOfflineNode btw .. nats transporter doesn't have an addOfflineNode method. which makes me think this won't respect instance_ip at all

nvm we shouldn't need instance_ip then anywayg

@debdutdeb
Copy link
Member

i'll give it a try now

@debdutdeb debdutdeb force-pushed the add-support-for-different-monolith-transporters branch from 5dd29af to 390de59 Compare May 29, 2023 21:09
@ggazzo ggazzo modified the milestones: 6.3.0, 6.2.3 May 30, 2023
@dionisio-bot dionisio-bot bot added stat: ready to merge PR tested and approved waiting for merge and removed stat: needs QA labels Jun 2, 2023
@sampaiodiego sampaiodiego merged commit 3109a76 into develop Jun 2, 2023
39 checks passed
@sampaiodiego sampaiodiego deleted the add-support-for-different-monolith-transporters branch June 2, 2023 20:35
sampaiodiego added a commit that referenced this pull request Jun 2, 2023
Co-authored-by: Debdut Chakraborty <debdut.chakraborty@rocket.chat>
@sampaiodiego sampaiodiego changed the title feat: add support for MONOLITH_TRANSPORTER env var feat: Add support to use TRANSPORTER env var for monolith deployments Jun 5, 2023
aleksandernsilva pushed a commit that referenced this pull request Jun 6, 2023
Co-authored-by: Debdut Chakraborty <debdut.chakraborty@rocket.chat>
gabriellsh added a commit that referenced this pull request Jun 6, 2023
…importer-messages-not-shown

* 'develop' of github.com:RocketChat/Rocket.Chat:
  fix: Room history scroll position (#29335)
  fix: check for $addToSet to be not empty before passing update params (#29378)
  chore: update `badge-level-0` color (#29460)
  fix: Saving Business hour throws an alert (#29449)
  regression: emojiPicker position (#29408)
  chore: Update codeowners to add ownership of rest typings package to backend (#29437)
  fix: Removed agent access to already taken rooms (#28979)
  fix: `queuedForUser` endpoint not filtering by status (#29189)
  regression: Marketplace Selectors (#29443)
  chore: fix contextualbar size (#29441)
  chore: `ComposerBoxPopup` Items alignment (#29320)
  feat: add support for `MONOLITH_TRANSPORTER` env var (#29373)
  fix: Import progress page stuck at 0% (#29421)
  feat: access-marketplace permission (#29203)
  chore(ddp-client): freeze data emitted by the sdk (#29430)
  refactor: `Table` on Integrations Page (#29428)
  regression: Update `LoadingIndicator` colors (#29424)
  chore(meteor): Update mention style (#29426)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stat: QA skipped stat: ready to merge PR tested and approved waiting for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants