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

Provide a way to get notified if entries are not found when watching #610

Merged
merged 2 commits into from
Nov 25, 2021

Conversation

minwoox
Copy link
Contributor

@minwoox minwoox commented Jun 11, 2021

Motivation:
Related #532
Currently, when we try to watch a file that doesn't exist,
the watch API waits until the file is added or timeout happens.
It sometimes makes user difficult to debug because the client cannot distinguish such cases.

Modifications:

Result:

  • You can now get notified if the entry doesn't exist when watching.

To-do:

  • Add the option to client-side

Motivation:
Related line#532
Currently, when we try to watch a file which doesn't exist,
the watch API waits until the file is added or timeout happens.
It sometimes makes user difficult to debug because the client cannot distinguish such cases.

Modifications:
- Add `notify-entry-not-found` preference to the watch `Prefer` header.
  - https://datatracker.ietf.org/doc/html/rfc7240#section-4

Result:
- You can now get notified if the entry doesn't exist when watching.

To-do:
- Add the option to client-side
@minwoox minwoox added this to the 0.51.0 milestone Jun 11, 2021
@minwoox minwoox requested review from ikhoon and trustin as code owners June 11, 2021 15:22
@codecov
Copy link

codecov bot commented Jun 11, 2021

Codecov Report

Merging #610 (6441934) into master (dd4b13d) will increase coverage by 0.03%.
The diff coverage is 79.16%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #610      +/-   ##
============================================
+ Coverage     69.88%   69.91%   +0.03%     
- Complexity     3281     3291      +10     
============================================
  Files           332      332              
  Lines         13036    13097      +61     
  Branches       1404     1416      +12     
============================================
+ Hits           9110     9157      +47     
- Misses         3062     3069       +7     
- Partials        864      871       +7     
Impacted Files Coverage Δ
...internal/storage/repository/RepositoryWrapper.java 31.25% <0.00%> (ø)
...raldogma/server/storage/repository/Repository.java 62.93% <50.00%> (-0.79%) ⬇️
...internal/storage/repository/git/GitRepository.java 75.75% <68.42%> (-0.21%) ⬇️
...e/repository/cache/CacheableFindLatestRevCall.java 72.72% <80.00%> (-4.55%) ⬇️
.../internal/api/converter/WatchRequestConverter.java 76.78% <81.25%> (+11.16%) ⬆️
...raldogma/server/internal/api/ContentServiceV1.java 85.38% <100.00%> (+0.88%) ⬆️
...centraldogma/server/internal/api/WatchService.java 76.27% <100.00%> (+3.38%) ⬆️
...al/storage/repository/cache/CachingRepository.java 91.17% <100.00%> (+1.86%) ⬆️
...erver/internal/thrift/CentralDogmaServiceImpl.java 74.65% <100.00%> (ø)
...ogma/server/storage/repository/RepositoryUtil.java 87.14% <100.00%> (+0.57%) ⬆️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dd4b13d...6441934. Read the comment docs.

Copy link
Contributor

@ikhoon ikhoon left a comment

Choose a reason for hiding this comment

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

Thanks! @minwoox

@minwoox minwoox modified the milestones: 0.51.0, 0.52.0 Jun 28, 2021
@minwoox minwoox modified the milestones: 0.52.0, 0.52.1 Aug 24, 2021
@minwoox minwoox requested a review from jrhee17 as a code owner November 25, 2021 07:35
@minwoox minwoox merged commit 0ee3aab into line:master Nov 25, 2021
@minwoox minwoox deleted the watchOption branch November 25, 2021 08:04
@minwoox
Copy link
Contributor Author

minwoox commented Nov 25, 2021

Thanks for reviewing. 😉

@ikhoon ikhoon added this to the 0.53.0 milestone Dec 3, 2021
minwoox pushed a commit that referenced this pull request Dec 9, 2021
### Motivation
- #532
- To-Do of #610

### Modifications
- Put value of `notify-entry-not-found` on `Prefer` request header to get error that entries are not found on watching file/repository
- Propagate error that entries are not found into `Watcher#initialValueFuture`

### Result
- You can get error on watching file/repository if the entry doesn't exist.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants