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

puppetboard: Adding PuppetDB 3.x support #176

Merged
merged 6 commits into from
Nov 10, 2015
Merged

puppetboard: Adding PuppetDB 3.x support #176

merged 6 commits into from
Nov 10, 2015

Conversation

corey-hammerton
Copy link
Contributor

Some excerpts from CHANGELOG.rst include:

  • Increasing the pypuppetdb requirements from 0.1.x to 0.2.x
  • The Reports page now lists reports from the reports endpoint instead of
    a link to a PuppetDB issue with a feature request
  • Adding a Catalogs page to view either individual node catalogs or compare
    them against other nodes
  • New environment awareness adds a new query parameter to all applicable
    endpoints to filter results based on the current environment. If the
    default environment 'production' is not available, or any other unavailable
    environment, the user is redirected to the first known environment.
  • Adding pagination functionality for reports (for now) based on the value of
    the REPORTS_COUNT configuration option (used for the limit and the offset
    calculation). Implementation also makes it possible for other UI enhancements.
  • Removing the limit_reports function from puppetboard/utils.py since paging
    parameters are now accepted by the pypuppetdb endpoint functions.
  • Bumping the version to 0.1.0

Some excerpts from CHANGELOG.rst include:
- Increasing the pypuppetdb requirements from 0.1.x to 0.2.x
- The Reports page now lists reports from the reports endpoint instead of
  a link to a PuppetDB issue with a feature request
- Adding a Catalogs page to view either individual node catalogs or compare
  them against other nodes
- New environment awareness adds a new query parameter to all applicable
  endpoints to filter results based on the current environment. If the
  default environment 'production' is not available, or any other unavailable
  environment, the user is redirected to the first known environment.
- Adding pagination functionality for reports (for now) based on the value of
  the REPORTS_COUNT configuration option (used for the limit and the offset
  calculation). Implementation also makes it possible for other UI enhancements.
- Removing the limit_reports function from puppetboard/utils.py since paging
  parameters are now accepted by the pypuppetdb endpoint functions.
- Bumping the version to 0.1.0
$('#switch_env').change(function() {
path = location.pathname.split('/');
path[1] = $(this).find(':selected').text();
location.assign(path.join('/'))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I prefer a bootstrap-like dropdown in the navbar with the hyperlinks directly setup. Am planning on (re-)implementing bootstrap after the 0.1.0 release.

Copy link
Member

Choose a reason for hiding this comment

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

Semantic-UI supports dropdown in many places just fine. It'd be nice to have 1 framework that we use though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's awesome @daenney, I would still like to work on bootstrap to have the added benefit of being mobile friendly. But then again I don't know Semantic UI that well either...

Copy link
Member

Choose a reason for hiding this comment

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

Same thing again. Semantic UI is perfectly mobile friendly though we're on a pretty old version as it currently stands. Feel free to change it around though but do stick to 1 framework for everything, otherwise it becomes unnecessarily complex.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed

corey-hammerton added a commit that referenced this pull request Oct 27, 2015
Make Puppetboard work with the PuppetDB V4 api

This PR just enables the basic functionality of PuppetBoard with the new PyPuppetDB version. #176 includes this work as well as other work to make better utilization of the new library
@corey-hammerton corey-hammerton added this to the 0.1.0 milestone Oct 28, 2015
@corey-hammerton
Copy link
Contributor Author

This PR also includes #161, after I iron out the issues with that I will include those changes here since this PR affects PuppetDB queries and template rendering.

Removing the environment switching point in the Known Issues section that has been dealt with.
@corey-hammerton
Copy link
Contributor Author

Tonight or Saturday I will fix these merge conflicts then we can have this PR merged.

After that I can remove the footer and maybe one or 2 more things then release 0.1.0. Cool?

@corey-hammerton corey-hammerton mentioned this pull request Nov 5, 2015
Fixing merge conflicts

Conflicts:
	puppetboard/app.py
	puppetboard/forms.py
	puppetboard/templates/catalogs.html
@corey-hammerton
Copy link
Contributor Author

If there are no more objections I will merge this PR either tonight or tomorrow.

envs = environments()

if env not in envs:
return redirect(url_for_environments(envs[0]))
Copy link
Member

Choose a reason for hiding this comment

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

This should give a 404 imho. You're requesting an environment that's not there. Just blindly redirecting to the first entry in envs is surprising.

envs = environments()

if env not in envs:
return redirect(url_for_environments(envs[0]))
Copy link
Member

Choose a reason for hiding this comment

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

Same as above.

@daenney
Copy link
Member

daenney commented Nov 6, 2015

Considering this line of code appears a whopping 16 times:

env = environments()
if env not in envs:
     return redirect(url_for_environments(envs[0]))

Extract that into a function that does the correct thing and just call that one function. That keeps the functionality in one place so updating the behaviour will be correctly reflected by everything that uses it.

@corey-hammerton
Copy link
Contributor Author

Despite some of the caveats and frustrations I love code/peer reviews 😛

…nd checking

Moving the envs variable out of the functions scope to the global scope,
this enables each function to reference and use these values.

Adding a new function check_env() to standardize the way to ensure that
the request environment exists, if it doesn't then abort with a 404.

This reduces 16 blocks of code and is in line with @daenney's suggestions
@corey-hammerton
Copy link
Contributor Author

Those 16 blocks of environment code have been replaced with a global scope envs variable and the function check_env(env) that all URL endpoints now call.

@daenney
Copy link
Member

daenney commented Nov 8, 2015

👍

@corey-hammerton
Copy link
Contributor Author

@daenney If all is well now I would like to merge this PR tonight so that we can hopefully release PuppetBoard 0.1.0.

@raphink
Copy link
Member

raphink commented Nov 9, 2015

👍 looking forward!

corey-hammerton added a commit that referenced this pull request Nov 10, 2015
puppetboard: Adding PuppetDB 3.x support

Some of the implementations here is:

- Environment Awareness with a dropdown menu to switch environments
- First implementation of pagination in the form of a macro for flexibility
- Use of the PuppetDB 3.x APIs, therefore PuppetDB 2.x compatibility is broken
@corey-hammerton corey-hammerton merged commit 709480a into voxpupuli:master Nov 10, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants