-
Notifications
You must be signed in to change notification settings - Fork 38
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
[MM-56920] Allow passing SiteURL override for jobs #640
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #640 +/- ##
========================================
+ Coverage 9.30% 9.34% +0.04%
========================================
Files 26 26
Lines 5308 5316 +8
========================================
+ Hits 494 497 +3
- Misses 4761 4767 +6
+ Partials 53 52 -1 ☔ View full report in Codecov by Sentry. |
// initialize some basic state. | ||
await Promise.all([ | ||
getMe()(store.dispatch, store.getState), | ||
getMyPreferences()(store.dispatch, store.getState), | ||
getMyTeams()(store.dispatch, store.getState), | ||
getMyTeamMembers()(store.dispatch, store.getState), | ||
]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Slightly unrelated but when investigating the flow of requests I figured these were not needed for the recorder. In fact they would all fail as unauthorized since the bot wouldn't be logged in. So we are moving them on the global widget logic .
standalone/src/widget/index.tsx
Outdated
getMe()(store.dispatch, store.getState), | ||
getMyPreferences()(store.dispatch, store.getState), | ||
getMyTeams()(store.dispatch, store.getState), | ||
getMyTeamMembers()(store.dispatch, store.getState), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I think all of these can be done like in like 84, e.g. store.dispatch(getMe())
. But no big deal either way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh yeah, after years I am still confused by actions. I'll update.
Summary
By default our jobs use the SiteURL from config in order to connect. To allow connection from within a local network in production we need to add some overrides.
Related PR
mattermost/calls-recorder#64
Ticket Link
https://mattermost.atlassian.net/browse/MM-56920