-
Notifications
You must be signed in to change notification settings - Fork 236
feat: expectation pattern support #2941
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
Conversation
90bebc4 to
ac807a7
Compare
6b9d9a3 to
4da5d2b
Compare
ca4c8a2 to
c3c33bc
Compare
838d1a3 to
4c1de74
Compare
cfde8f5 to
603ac9d
Compare
.../src/main/java/io/javaoperatorsdk/operator/processing/expectation/RegisteredExpectation.java
Show resolved
Hide resolved
...src/test/java/io/javaoperatorsdk/operator/processing/expectation/ExpectationManagerTest.java
Show resolved
Hide resolved
...src/test/java/io/javaoperatorsdk/operator/processing/expectation/ExpectationManagerTest.java
Show resolved
Hide resolved
...src/test/java/io/javaoperatorsdk/operator/processing/expectation/ExpectationManagerTest.java
Outdated
Show resolved
Hide resolved
| return UpdateControl.noUpdate(); | ||
| } | ||
| } else { | ||
| // checks the expectation if it is fulfilled also removes it, |
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.
same fix as in the docs
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.
not sure what do you mean... pls ellaborate
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.
basically I wrote everything the code in the docs - #2941 (comment), so just do the same in the original code that the docs refference.
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 still not fixed, but leave it be, I'll fix it after the PR is merged
|
thank you @xstefank for all the fixes in javadocs and other suggestions!! |
4149721 to
fda19a2
Compare
Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
Co-authored-by: Martin Stefanko <xstefank122@gmail.com>
Co-authored-by: Martin Stefanko <xstefank122@gmail.com>
Co-authored-by: Martin Stefanko <xstefank122@gmail.com>
Co-authored-by: Martin Stefanko <xstefank122@gmail.com>
Co-authored-by: Martin Stefanko <xstefank122@gmail.com>
Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
fda19a2 to
98f92ba
Compare
I just want to test if this really allows me to commit to your PR branch. It's not a big change.
xstefank
left a comment
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.
@csviri still a few things that need to be looked at
...ework-core/src/main/java/io/javaoperatorsdk/operator/processing/expectation/Expectation.java
Show resolved
Hide resolved
...core/src/main/java/io/javaoperatorsdk/operator/processing/expectation/ExpectationStatus.java
Show resolved
Hide resolved
...va/io/javaoperatorsdk/operator/processing/expectation/PeriodicCleanerExpectationManager.java
Outdated
Show resolved
Hide resolved
...va/io/javaoperatorsdk/operator/processing/expectation/PeriodicCleanerExpectationManager.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
…tor/processing/expectation/PeriodicCleanerExpectationManager.java Co-authored-by: Martin Stefanko <xstefank122@gmail.com>
Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
| return UpdateControl.noUpdate(); | ||
| } | ||
| } else { | ||
| // checks the expectation if it is fulfilled also removes it, |
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 still not fixed, but leave it be, I'll fix it after the PR is merged
Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
See related issue