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

Show puppetdb connection error message on homepage #692

Merged
merged 9 commits into from
Jun 8, 2022
Merged

Show puppetdb connection error message on homepage #692

merged 9 commits into from
Jun 8, 2022

Conversation

ArthurWuTW
Copy link
Contributor

  • Able to show error message on homepage when puppetdb connection failed.
    image

  • Able to show 0 nodes on homepage without any agents yet.
    image

@codecov
Copy link

codecov bot commented Jun 6, 2022

Codecov Report

Merging #692 (71afad4) into master (1c5a719) will decrease coverage by 0.36%.
The diff coverage is 80.00%.

@@            Coverage Diff             @@
##           master     #692      +/-   ##
==========================================
- Coverage   86.29%   85.93%   -0.37%     
==========================================
  Files          19       19              
  Lines        1051     1059       +8     
==========================================
+ Hits          907      910       +3     
- Misses        144      149       +5     
Impacted Files Coverage Δ
puppetboard/views/query.py 86.04% <50.00%> (-2.05%) ⬇️
puppetboard/app.py 91.48% <100.00%> (+0.18%) ⬆️
puppetboard/views/index.py 95.00% <100.00%> (+0.08%) ⬆️
puppetboard/views/failures.py 26.19% <0.00%> (-2.76%) ⬇️
puppetboard/views/reports.py 64.96% <0.00%> (+0.25%) ⬆️

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 1c5a719...71afad4. Read the comment docs.

@gdubicki
Copy link
Contributor

gdubicki commented Jun 6, 2022

This implementation is definitely an improvement, but if you would use the error handlers feature from Flask then it would work on any view, not just the Overview. Have you thought of that, @ArthurWuTW?

@ArthurWuTW
Copy link
Contributor Author

ArthurWuTW commented Jun 7, 2022

At first I wanted to make the minimal change to achieve those improvements for Overview only, but the error handler feature looks nice, I will check it out and give it a try :)

I'm new to codecov, what is the check of "codecov/project" for? it failed but I'm not really sure what the problem is...

@ArthurWuTW
Copy link
Contributor Author

ArthurWuTW commented Jun 7, 2022

I just found that it has already had error handlers in code base.
Just import errors.py and skip env_check when env=default_env(to show 0 nodes).

  • start puppetboard only - error message is clear enough
    image

  • start puppetboard and puppetdb with 0 nodes
    image
    image

  • start puppetboard and puppetdb with 1 node
    image

@gdubicki
Copy link
Contributor

gdubicki commented Jun 7, 2022

I just found that it has already had error handlers in code base.

Oups, I have really forgotten about it...

Well, anyway, it's great that we can achieve what we want with the minimal amount of changes that we have in this PR.

Can you also just please test if the Query view still behaves like it did? It's the only screen where we expect to return 400 or 404 in a normal day-to-day use, when a user enters an invalid query or it returns zero results. Please see #653 for a reference, if needed.

If we still show the user-friendly messages there then I think we can merge and release this.

@ArthurWuTW
Copy link
Contributor Author

ArthurWuTW commented Jun 7, 2022

before this commit 71afad4, when 0 nodes, Query was like...
image

if we need to be able to query when 0 nodes, we need to add the same condition to skip env_check as Overview, and Query behaves as normal.

image
image

@gdubicki
Copy link
Contributor

gdubicki commented Jun 8, 2022

Thanks @ArthurWuTW, I think we are good to go with this now. :)

@gdubicki gdubicki merged commit 550250b into voxpupuli:master Jun 8, 2022
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.

3 participants