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

CE-657 - Move Application leader election tutorial to docs #21366

Merged
merged 18 commits into from
Aug 15, 2024
Merged

Conversation

danielehc
Copy link
Contributor

@danielehc danielehc commented Jun 25, 2024

Description

Preview: https://consul-git-ce-657-hashicorp.vercel.app/consul/docs/dynamic-app-config/sessions

Testing & Reproduction steps

Links

PR Checklist

  • updated test coverage
  • external facing docs updated
  • appropriate backport labels added
  • not a security concern

@danielehc danielehc added type/docs Documentation needs to be created/updated/clarified pr/no-changelog PR does not need a corresponding .changelog entry pr/no-backport labels Jun 25, 2024
@danielehc danielehc self-assigned this Jun 25, 2024
@danielehc danielehc requested a review from a team as a code owner June 25, 2024 09:26
@danielehc danielehc changed the title Move Application leader election tutorial to docs CE-657 - Move Application leader election tutorial to docs Jun 25, 2024
Copy link
Contributor

@aimeeu aimeeu left a comment

Choose a reason for hiding this comment

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

style guide nits

Comment on lines 137 to 138
The primitives provided by sessions and the locking mechanisms of the KV
store can be used to build client-side leader election algorithms.
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
The primitives provided by sessions and the locking mechanisms of the KV
store can be used to build client-side leader election algorithms.
You can use the primitives provided by sessions and the locking mechanisms of the KV
store to build client-side leader election algorithms.

Copy link
Contributor

@boruszak boruszak left a comment

Choose a reason for hiding this comment

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

Excellent work! This doc is looking good.

I left an initial round of suggestions. After you implement them, ask Aimee to give her review.

Comment on lines 221 to 225
<Note>

This locking system is purely advisory. There is no enforcement that clients must acquire a lock to perform any operation. Any client can read, write, and delete a key without owning the corresponding lock. It is not the goal of Consul to protect against misbehaving clients.

</Note>
Copy link
Contributor

Choose a reason for hiding this comment

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

Odd placement for this note

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought that, after learning how to perform the lock it was useful to get a remark that the process is different from an actual lock that would prevent other operations to be performed. Do you have suggestions over which position would fit better?

Copy link
Contributor

@boruszak boruszak left a comment

Choose a reason for hiding this comment

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

Final suggestions and style issues that I noticed.

Looks excellent. Merge when ready!

@danielehc danielehc merged commit e2bb1b7 into main Aug 15, 2024
58 of 59 checks passed
@danielehc danielehc deleted the CE-657 branch August 15, 2024 07:07
boruszak added a commit that referenced this pull request Aug 27, 2024
* First commit

* Fix navigation

* Add some commands

* Structure draft

* Complete usage doc structure

* Fix link

* Apply suggestions from code review

Co-authored-by: Aimee Ukasick <[email protected]>

* Apply suggestions from code review

Co-authored-by: Jeff Boruszak <[email protected]>

* Apply suggestions from code review

Co-authored-by: Jeff Boruszak <[email protected]>

* Apply suggestions from code review

* Replace tutorial path

* Apply suggestions from code review

Co-authored-by: Jeff Boruszak <[email protected]>

---------

Co-authored-by: Aimee Ukasick <[email protected]>
Co-authored-by: Jeff Boruszak <[email protected]>
Co-authored-by: boruszak <[email protected]>
boruszak added a commit that referenced this pull request Aug 27, 2024
…21673)

* CE-655 - Moving DNS forwading tutorial to docs (#21348)

* First commit

* Add page to navigation

* test new doc page

* Update website/content/docs/services/discovery/dns-forwarding.mdx

* Update website/content/docs/services/discovery/dns-forwarding.mdx

* fix push build atttempt

* Draft

* Draft

* empty line

* Draft

* empty lines

* Draft

* First draft

* Create documentation for Argo Rollouts Plugin. (#20680)

* Create documentation for Argo Rollouts Plugin.

* Create documentation for Argo Rollouts Plugin.

* Apply suggestions from code review

Co-authored-by: David Yu <[email protected]>

* Apply suggestions from code review

Co-authored-by: Jeff Boruszak <[email protected]>

* Update docs based on feedback

* Apply suggestions from code review

Co-authored-by: Jeff Boruszak <[email protected]>

* Update website/content/docs/k8s/deployment-configurations/argo-rollouts-configuration.mdx

* Update website/content/docs/k8s/deployment-configurations/argo-rollouts-configuration.mdx

---------

Co-authored-by: David Yu <[email protected]>
Co-authored-by: Jeff Boruszak <[email protected]>
Co-authored-by: Michael Wilkerson <[email protected]>

* Split content and add images

* Fix navigation

* Add links and context

* Restructure changes

* Fix enable documentation

* Fix enable documentation

* Fix index documentation

* Add troubleshooting and fix codeblocks

* Add troubleshooting and fix codeblocks

* Typos and last checks

* Apply suggestions from code review

Co-authored-by: Aimee Ukasick <[email protected]>

* Apply suggestions from code review

Co-authored-by: Jeff Boruszak <[email protected]>

* Update website/content/docs/services/discovery/dns-forwarding/enable.mdx

* Apply suggestions from code review

Co-authored-by: Jeff Boruszak <[email protected]>

* Add dark mode images

* Add dark mode images

* Apply suggestions from code review

---------

Co-authored-by: boruszak <[email protected]>
Co-authored-by: Ashwin Venkatesh <[email protected]>
Co-authored-by: David Yu <[email protected]>
Co-authored-by: Jeff Boruszak <[email protected]>
Co-authored-by: Michael Wilkerson <[email protected]>
Co-authored-by: Aimee Ukasick <[email protected]>

* CE-657 - Move Application leader election tutorial to docs (#21366)

* First commit

* Fix navigation

* Add some commands

* Structure draft

* Complete usage doc structure

* Fix link

* Apply suggestions from code review

Co-authored-by: Aimee Ukasick <[email protected]>

* Apply suggestions from code review

Co-authored-by: Jeff Boruszak <[email protected]>

* Apply suggestions from code review

Co-authored-by: Jeff Boruszak <[email protected]>

* Apply suggestions from code review

* Replace tutorial path

* Apply suggestions from code review

Co-authored-by: Jeff Boruszak <[email protected]>

---------

Co-authored-by: Aimee Ukasick <[email protected]>
Co-authored-by: Jeff Boruszak <[email protected]>
Co-authored-by: boruszak <[email protected]>

---------

Co-authored-by: danielehc <[email protected]>
Co-authored-by: Ashwin Venkatesh <[email protected]>
Co-authored-by: David Yu <[email protected]>
Co-authored-by: Michael Wilkerson <[email protected]>
Co-authored-by: Aimee Ukasick <[email protected]>
philrenaud pushed a commit that referenced this pull request Sep 12, 2024
* First commit

* Fix navigation

* Add some commands

* Structure draft

* Complete usage doc structure

* Fix link

* Apply suggestions from code review

Co-authored-by: Aimee Ukasick <[email protected]>

* Apply suggestions from code review

Co-authored-by: Jeff Boruszak <[email protected]>

* Apply suggestions from code review

Co-authored-by: Jeff Boruszak <[email protected]>

* Apply suggestions from code review

* Replace tutorial path

* Apply suggestions from code review

Co-authored-by: Jeff Boruszak <[email protected]>

---------

Co-authored-by: Aimee Ukasick <[email protected]>
Co-authored-by: Jeff Boruszak <[email protected]>
Co-authored-by: boruszak <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/no-backport pr/no-changelog PR does not need a corresponding .changelog entry type/docs Documentation needs to be created/updated/clarified
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants