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.

Sign up for GitHub

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

Jump to bottom

Deployment Section Restructure #2 #7852

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Deployment Section Restructure #2 #7852

wants to merge 12 commits into from
+207 −253

Conversation

DSchalla
Copy link
Member

@DSchalla DSchalla commented Apr 8, 2025

Summary

Follow up to #7816, which introduces multiple bug fixes and enhancements.

@DSchalla DSchalla requested a review from spirosoik April 8, 2025 09:10
@github-actions GitHub Actions
Copy link

github-actions bot commented Apr 8, 2025

Newest code from mattermost has been published to preview environment for Git SHA 63fc34b

@github-actions GitHub Actions
Copy link

github-actions bot commented Apr 8, 2025

Newest code from mattermost has been published to preview environment for Git SHA 1daeb3d

@github-actions GitHub Actions
Copy link

github-actions bot commented Apr 8, 2025

Newest code from mattermost has been published to preview environment for Git SHA fa9f367

@wiersgallak wiersgallak requested a review from esethna April 8, 2025 16:10
esethna
esethna reviewed Apr 8, 2025
View reviewed changes
source/deploy/try-mattermost.rst Outdated
Deployment Options
------------------

.. tab:: Azure Marketplace
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we put Docker first since it's probably the most accessible way to quick start for an evaluation?

Following up on my comment in Hub, is there any specific reason the docker preview image is TE or can we make it E0?

Copy link
Member Author

Choose a reason for hiding this comment

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

Adding @spirosoik on both asks, good questions.

Copy link
Member

Choose a reason for hiding this comment

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

We should add the docker preview. I don't have a clear answer why docker preview is TE, I can discuss with the team

Copy link
Member Author

Choose a reason for hiding this comment

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

Docker Preview is currently added, the question I think is what should be the default tab. Currently it's Azure AIO VM.

esethna
esethna requested changes Apr 8, 2025
View reviewed changes
@github-actions GitHub Actions
Copy link

Newest code from mattermost has been published to preview environment for Git SHA f082be4

@DSchalla
Copy link
Member Author

@esethna Good notes - I'll update the pages later today and re-request review. Thanks!

cwarnermm
cwarnermm reviewed Apr 10, 2025
View reviewed changes
source/deploy/server/linux/deploy-omnibus.rst
@@ -27,6 +27,9 @@

</div>

.. warning::
Copy link
Member

Choose a reason for hiding this comment

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

In addition to this docs warning, a Forum announcement post is also required.

Copy link
Member Author

Choose a reason for hiding this comment

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

@spirosoik Did you discuss this one with PM/CS already? Would you be open to prepare the forum post?

@github-actions GitHub Actions
Copy link

Newest code from mattermost has been published to preview environment for Git SHA 608f7bf

@github-actions GitHub Actions
Copy link

Newest code from mattermost has been published to preview environment for Git SHA 716ed9b

@DSchalla
Copy link
Member Author

Updated the PRs, outstanding organizational questions re: deprecation notice and preview orders, but otherwise good to go for second review.

Spiros will prepare a second PR for other changes.

cwarnermm
cwarnermm reviewed Apr 10, 2025
View reviewed changes
source/deploy/server/preparations.rst Outdated

When using an HTTPS proxy, ensure your Mattermost server has the proxy's root certificate configured to avoid connection issues.

.. dropdown:: Example systemd Service Configuration
Copy link
Member

Choose a reason for hiding this comment

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

The current Sphinx environment doesn't support the .. dropdown:: directive. As a result, no content associated with this directive is displayed when the docs are generated.

Are you looking to collapse and expand content on the page or something else?

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct, I'd like this to be collapsed and optionally expanded if the user would like to, but different directives didn't work. Any recommendation?

Copy link
Member

Choose a reason for hiding this comment

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

Yes. We'd need a Sphinx extension to support collapse/expand functionality in our environment. I have one in mind. Is this a must-have to get this PR in, or a nice-to-have we could iterate on over time?

Copy link
Member Author

Choose a reason for hiding this comment

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

Definitely a nice-to-have - We can just make it a code block instead.

@cwarnermm
Copy link
Member

@DSchalla - the following build warnings are present via this docs PR. In all 3 cases, the source file isn't defined as a nav pane item. Can I get your help to confirm whether this is intentional or not, please?

  1. configure/configuring-cloudfront-to-host-mattermost-static-assets.rst -- Has this content been accounted for somewhere? Is the source file still needed?
  2. configure/using-outbound-proxy.rst -- Has the content been moved to deploy/server/preparations.html#outbound-proxy-configuration? Is the source file still needed?
  3. /deploy/server/setting-up-socket-based-mattermost-database.rst -- Has this content been accounted for somewhere? Is the source file still needed?

@github-actions GitHub Actions
Copy link

Newest code from mattermost has been published to preview environment for Git SHA f7ef0f8

@DSchalla
Copy link
Member Author

Good questions.

  1. Intentionally removed during a conversation with Spiros - I'd advise making this a KB article. That's a very edge-case scenario I'd rarely advise customers to use.
  2. Correct, source file not needed as content was moved.
  3. See 1).

Thanks for your help, Carrie!

@github-actions GitHub Actions
Copy link

Newest code from mattermost has been published to preview environment for Git SHA c724207

@cwarnermm
Copy link
Member

cwarnermm commented Apr 14, 2025 •
edited
Loading

Build errors resolved, redirects added, & ..dropdown:: directive removed

@cwarnermm
Copy link
Member

@DSchalla - Are more content/changes expected that will need review?

This PR is ready for merge from my perspective. Please feel free to merge when ready.

@cwarnermm cwarnermm self-requested a review April 14, 2025 14:17
@github-actions GitHub Actions
Copy link

Newest code from mattermost has been published to preview environment for Git SHA 5f942f8

@DSchalla
Copy link
Member Author

@cwarnermm Thank you, negative, was largely pending on @esethna and @wiersgallak comment.

@spirosoik See the question on Omnibus and Docker vs. Azure for Trial.

Katie, Eric, let me know if you have any pending questions. Otherwise I'll merge tomorrow morning.

@github-actions GitHub Actions
Copy link

Newest code from mattermost has been published to preview environment for Git SHA 0c9db0c

esethna
esethna reviewed Apr 14, 2025
View reviewed changes
source/deploy/quick-start-evaluation.rst
5. Once deployed, access your Mattermost instance using the public IP address or DNS name provided.

.. note::
The Azure Marketplace image comes with PostgreSQL and is more suitable for testing production-like scenarios. Remember to delete the resources when you're done to avoid unnecessary charges.
Copy link
Contributor

Choose a reason for hiding this comment

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

In the doc intro blurb it states that both these deploy options use SQLite databases so are not configured for production. It this statement here contradictory? @DSchalla

Copy link
Member Author

Choose a reason for hiding this comment

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

@nickmisasi @toninis Can you help here as I don't have much background with the AIO image?

@esethna
Copy link
Contributor

esethna commented Apr 14, 2025

@DSchalla added one comment above,

re Azure vs Docker being the default (and if the docker image can be E0), we don't need to block the PR since @spirosoik is out for the week, we can follow-up when he's back

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@esethna esethna esethna requested changes

@cwarnermm cwarnermm cwarnermm approved these changes

@spirosoik spirosoik Awaiting requested review from spirosoik

@wiersgallak wiersgallak Awaiting requested review from wiersgallak

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@DSchalla @cwarnermm @esethna @spirosoik @wiersgallak