-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
docs: in-line versioning + deprecation for multi-semaphore #13645
base: main
Are you sure you want to change the base?
docs: in-line versioning + deprecation for multi-semaphore #13645
Conversation
Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
…d was not read either Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
…al IMO Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
@@ -24,10 +24,6 @@ data: | |||
template: "2" # Two instances of template can run at a given time in particular namespace | |||
``` | |||
|
|||
!!! Warning | |||
Each synchronization block may only refer to either a semaphore or a mutex. | |||
If you specify both only the semaphore will be locked. |
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.
No longer relevant in 3.6
... | ||
``` | ||
|
||
Specifying both would not work in <3.6, only the semaphore would be used. |
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.
We just need to fix this bug per #13358 (comment) and #11859 -- i.e. give a validation error instead of silently ignoring it in 3.5 and prior versions
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.
Fixed with a validation error in #13669
|
||
The single `mutex` and `semaphore` syntax still works in version 3.6 but is considered deprecated. | ||
Both the `mutex` and the `semaphore` will be taken in version 3.6 with this syntax. | ||
This syntax can be mixed with `mutexes` and `semaphores`, all locks will be required. |
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.
if we want to describe this, this would be better described with another example
name: welcome | ||
# mutexes: # v3.6 and after | ||
# - name: welcome |
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.
I'm inclined to just move these to internal tests and otherwise remove the legacy examples entirely from the user-facing examples/
dir
and flipped syntax + in-line comments here is a stopgap, especially as the in-line deprecation is hard to understand without the future syntax right next to it (and users are likely to miss -legacy
name too)
Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
- [`synchronization-mutex-wf-level-legacy.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/synchronization-mutex-wf-level-legacy.yaml) | ||
|
||
- [`synchronization-mutex-wf-level.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/synchronization-mutex-wf-level.yaml) |
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.
this is suboptimal as the field checker automation is plain text search and so does not recognize that it's in a comment, but nbd in context as it does show it and say it's deprecated
Addresses #13644, #13358 (comment) etc:
And ofc this contributor thread and previous Contributor Meeting with Alan and I butting heads mostly over in-line version specifiers 😅
Motivation
Mitigate users not realizing they're on the
latest
version of the docs and attempting to use features that don't exist yet, getting errors, and then filing issues or support requests about it.To some extent, it is inevitable, but in-line versioning is a mitigation in an effort to help users self-service.
Modifications
remove warning about using mutexes and semaphores together as that is possible in 3.6 after feat: multiple mutexes and semaphores #13358 -- the warning should only be in earlier versions
use snippet embeds to directly use examples in the docs without duplication
whalesay
image withbusybox
#13429 (comment) and elsewhere, thought this was a good time to start using themadd in-line comments with version specifiers on
mutexes
andsemaphores
syntaxadd deprecation comments with version specifiers on
mutex
andsemaphore
syntaxadd alternative syntax in comments, as suggested in feat: multiple mutexes and semaphores #13358 (comment)
remove "Legacy" section as this does not match conventions
semaphores
error:cannot get LockName for a Sync of Unknown type
#13644 et almutex
withsemaphore
and it not erroring out but silently ignoring themutex
is a confusing bug.Verification
make docs
passes.Here's a screenshot of the example snippet embeds:
Future Work