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

feat: STALE state, run handlers for current state immediately, provider name #196

Merged
merged 9 commits into from
Aug 15, 2023
Merged

feat: STALE state, run handlers for current state immediately, provider name #196

merged 9 commits into from
Aug 15, 2023

Conversation

toddbaert
Copy link
Member

@toddbaert toddbaert commented Jun 23, 2023 •
edited
Loading

  • Adds additional clarity regarding provider/client name mapping, as well as intent
  • Replaces client-name for provider-name in events-details
  • adds "STALE" state
  • handlers for ANY state run immediately if provider is in that state

The only functional changes here are small ones related to events, and a new provider state, so particularly interested in @Kavindu-Dodan and @lukas-reining 's opinions on this one.

Relates to #200, which depends on STALE state.

Kavindu-Dodan
Kavindu-Dodan reviewed Jun 23, 2023
View reviewed changes
specification.json Outdated Show resolved Hide resolved
beeme1mr
beeme1mr previously approved these changes Jul 12, 2023
View reviewed changes
@toddbaert toddbaert marked this pull request as draft July 20, 2023 16:14
@toddbaert toddbaert requested a review from beeme1mr July 20, 2023 16:51
@toddbaert toddbaert dismissed beeme1mr’s stale review July 20, 2023 16:52

Significant changes added

@toddbaert toddbaert mentioned this pull request Jul 24, 2023
Merged
toddbaert and others added 3 commits August 11, 2023 14:25
@toddbaert
fix: additional clarity, events fixes
4b8a81c 
* Adds additional clarity regarding provider/client name mapping, as well as intent.
* Requires that READY events only run immediately for clients when the provider is already ready.
* Replaces client-name for provider-name in events-details.

Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
@toddbaert @justinabrahms
Update specification/sections/01-flag-evaluation.md
7afa3d7 
Co-authored-by: Justin Abrahms <justin@abrah.ms>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
@toddbaert
fixup: getClient example fix
a65b7c4 
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
@toddbaert
fixup: revert 1.1.3
68278fc 
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
@toddbaert
Update specification/sections/05-events.md
6d261e5 
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
@toddbaert
Update specification/sections/05-events.md
d6a374f 
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
@toddbaert
fixup: parse again
3f56549 
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
@toddbaert
fixup: STALE state
0e38be8 
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
@toddbaert toddbaert marked this pull request as ready for review August 11, 2023 18:33
@toddbaert
Copy link
Member Author

@Kavindu-Dodan @lukas-reining @beeme1mr @Kavindu-Dodan I've updated this PR. Please re-review.

The main change is the addition of the STALE provider state, and changes to 5.3.3: instead of just READY events running immediately, now the spec dictates that handlers of any type should run immediately if they are added when the provider is already in the associated state.

lukas-reining
lukas-reining approved these changes Aug 11, 2023
View reviewed changes
Copy link
Member

@lukas-reining lukas-reining left a comment

Choose a reason for hiding this comment

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

Looks good to me :)

@toddbaert
fixup: typo
25fec7e 
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
@toddbaert toddbaert changed the title fix: additional clarity, events fixes feat: STALE state, run handlers for current state immediately, clarity Aug 15, 2023
@toddbaert toddbaert changed the title feat: STALE state, run handlers for current state immediately, clarity feat: STALE state, run handlers for current state immediately, provider name Aug 15, 2023
@toddbaert toddbaert merged commit 8e8c344 into main Aug 15, 2023
4 checks passed
@toddbaert toddbaert deleted the fix/clarity-and-events branch August 15, 2023 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@beeme1mr beeme1mr beeme1mr approved these changes

@Kavindu-Dodan Kavindu-Dodan Kavindu-Dodan approved these changes

@lukas-reining lukas-reining lukas-reining approved these changes

@benjiro benjiro Awaiting requested review from benjiro

@kinyoklion kinyoklion Awaiting requested review from kinyoklion

@thomaspoignant thomaspoignant Awaiting requested review from thomaspoignant

@thiyagu06 thiyagu06 Awaiting requested review from thiyagu06

@moredip moredip Awaiting requested review from moredip

@justinabrahms justinabrahms Awaiting requested review from justinabrahms

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
@toddbaert @justinabrahms @beeme1mr @Kavindu-Dodan @lukas-reining