Skip to content

Conversation

@ravikiran-sulikeri
Copy link
Collaborator

@ravikiran-sulikeri ravikiran-sulikeri commented Dec 11, 2023


If you open a PR that needs to go into a current version, you need to cherry-pick your commit from dev over to the current version branch. Only then will the proper builds that generate html/pdf be run. But beware: Docs will be generated but not published automatically!

  • N/A - or - I have added the appropriate "cherry-pick-to" labels to this PR so I don't forget to do this later!

Trello

@netlify
Copy link

netlify bot commented Dec 11, 2023

Deploy Preview for priceless-fermi-5883f3 ready!

Name Link
🔨 Latest commit c0ac9ac
🔍 Latest deploy log https://app.netlify.com/sites/priceless-fermi-5883f3/deploys/6582f1717ecaf1000861b480
😎 Deploy Preview https://deploy-preview-175--priceless-fermi-5883f3.netlify.app/ops-manager/1.8/addition/docker/container
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@neo-technology-commit-status-publisher
Copy link
Collaborator

neo-technology-commit-status-publisher commented Dec 11, 2023

Thanks for the documentation updates.

The preview documentation has now been torn down - reopening this PR will republish it.

Copy link
Contributor

@AlexicaWright AlexicaWright left a comment

Choose a reason for hiding this comment

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

Editorial and formatting comments and suggestions.

NEO4J_server_metrics_prometheus_endpoint="localhost:2004" \
NEO4J_server_metrics_filter="*" \
agent console -s'

Copy link
Collaborator

@alexnb alexnb Dec 18, 2023

Choose a reason for hiding this comment

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

I think security configuration variables are also needed, ex.

      CONFIG_TLS_TRUSTED_CERTS: "/certificates/server.cer"

EDIT: but maybe this one is not important since it is optional.

These properties are not needed for an agent (afaik):

NEO4J_ACCEPT_LICENSE_AGREEMENT="yes" 
NEO4J_EDITION="enterprise" 
NEO4J_server_metrics_prometheus_enabled="true" 
NEO4J_server_metrics_prometheus_endpoint="localhost:2004" 
NEO4J_server_metrics_filter="*" 

This would also apply to the other env variable blocks on this page.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

will update, thanks for introducing callouts

@alexnb
Copy link
Collaborator

alexnb commented Dec 18, 2023

Ravi, I changed some shell formatting, hope that is okay!

Copy link
Contributor

@lidiazuin lidiazuin left a comment

Choose a reason for hiding this comment

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

Left some editorial suggestions. Could you please take a look too @AlexicaWright ?

neo4j/neo4j:latest
----

<1> Neo4j home directory needs to be mounted back on the Docker host to enable access to agent.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<1> Neo4j home directory needs to be mounted back on the Docker host to enable access to agent.
Note that:
. The Neo4j home directory needs to be mounted back on the Docker host to enable access to agent.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think callouts make more sense in this section than a note. WDYT @AlexicaWright ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@ravikiran-sulikeri Ravi, fyi, there is currently an issue with callouts but I added them in anticipation that it gets fixed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Waiting for word on whether it's getting fixed in the near future. If so, I'd say keep the annotation.

----

<1> Neo4j home directory needs to be mounted back on the Docker host to enable access to agent.
<2> Neo4j prometheus endpoint port (2004) needs to be exposed via port mapping.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<2> Neo4j prometheus endpoint port (2004) needs to be exposed via port mapping.
. Neo4j Prometheus endpoint port (2004) needs to be exposed via port mapping.


<1> Neo4j home directory needs to be mounted back on the Docker host to enable access to agent.
<2> Neo4j prometheus endpoint port (2004) needs to be exposed via port mapping.
<3> Query log port (9500) needs to be mapped for log appender to forward query logs.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<3> Query log port (9500) needs to be mapped for log appender to forward query logs.
. Query log port (9500) needs to be mapped for log appender to forward query logs.

Copy link
Collaborator

@alexnb alexnb left a comment

Choose a reason for hiding this comment

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

LGTM!

neo4j/neo4j:latest
----

<1> Neo4j home directory needs to be mounted back on the Docker host to enable access to agent.
Copy link
Contributor

Choose a reason for hiding this comment

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

Waiting for word on whether it's getting fixed in the near future. If so, I'd say keep the annotation.

Co-authored-by: Jessica Wright <[email protected]>
Co-authored-by: Lidia Zuin <[email protected]>
Copy link
Contributor

@AlexicaWright AlexicaWright left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@ravikiran-sulikeri ravikiran-sulikeri merged commit 2b349b6 into neo4j:dev Dec 20, 2023
ravikiran-sulikeri added a commit to ravikiran-sulikeri/docs-ops-manager that referenced this pull request Dec 20, 2023
* Improves monitoring Neo4j standalone container docs

* minor edits

* Update modules/ROOT/pages/addition/docker/container.adoc

Co-authored-by: Jessica Wright <[email protected]>

* Update modules/ROOT/pages/addition/docker/container.adoc

Co-authored-by: Jessica Wright <[email protected]>

* Apply suggestions from code review

Co-authored-by: Jessica Wright <[email protected]>

* correct the doc formmating

* formatted shell commands

* combined 3 shell blocks into one with callouts

* fix agent env config scripts

* Apply suggestions from code review

Co-authored-by: Lidia Zuin <[email protected]>

* Update modules/ROOT/pages/addition/docker/container.adoc

Co-authored-by: Alexander Bouriakov <[email protected]>

* Apply suggestions from code review

Co-authored-by: Jessica Wright <[email protected]>
Co-authored-by: Lidia Zuin <[email protected]>

---------

Co-authored-by: Jessica Wright <[email protected]>
Co-authored-by: Alexander Bouriakov <[email protected]>
Co-authored-by: Lidia Zuin <[email protected]>
ravikiran-sulikeri added a commit that referenced this pull request Dec 20, 2023
… (#181)

Improves monitoring Neo4j standalone container docs (#175)

* Improves monitoring Neo4j standalone container docs

* minor edits

* Update modules/ROOT/pages/addition/docker/container.adoc



* Update modules/ROOT/pages/addition/docker/container.adoc



* Apply suggestions from code review



* correct the doc formmating

* formatted shell commands

* combined 3 shell blocks into one with callouts

* fix agent env config scripts

* Apply suggestions from code review



* Update modules/ROOT/pages/addition/docker/container.adoc



* Apply suggestions from code review




---------

Co-authored-by: Jessica Wright <[email protected]>
Co-authored-by: Alexander Bouriakov <[email protected]>
Co-authored-by: Lidia Zuin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants