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

Implement proposal 716, passing full paths through of-watchdog #21

Merged
merged 2 commits into from
Sep 6, 2018

Conversation

telackey
Copy link
Contributor

@telackey telackey commented Jul 25, 2018

An implementation of proposal #716, passing the full URL through the of-watchdog.

Description

Previous to these changes, only the query string provide by the HTTP client was passed through to the handling code. With these changes, the full URL path is passed as well as the query string.

Motivation and Context

  • I have raised an issue to propose this change

Proposal #716

How Has This Been Tested?

Manually.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Some of the templates have baked in routes of '/' (cf. the node8 template). Those will also need updated. The easiest will be to replace the explicit '/' route with a catchall:

-app.post('/', middleware);
-app.get('/', middleware);
+app.post('/*', middleware);
+app.get('/*', middleware);

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I've read the CONTRIBUTION guide
  • I have signed-off my commits with git commit -s
  • I have added tests to cover my changes.
  • All new and existing tests passed.

This may or may not require a documentation change. The existing documentation, AFAIK, does not address the use of paths one way or the other.

Refs

Trello: https://trello.com/c/1qZMlGXU

Previously, only the query string of the URL was passed through the of-watchdog.

With this change, the entire path requested by the client (after the function name) is passed through.

Signed-off-by: Thomas E Lackey <telackey@bozemanpass.com>
Signed-off-by: Thomas E Lackey <telackey@bozemanpass.com>
@alexellis
Copy link
Member

@telackey I'd like to see unit tests for this, but will merge and if there are any problems get in touch for a correction.

Alex

@alexellis alexellis merged commit c96acf7 into openfaas:master Sep 6, 2018
@telackey
Copy link
Contributor Author

telackey commented Sep 6, 2018

@alexellis Sounds good.

@alexellis
Copy link
Member

This has been released along with the gateway and the node template. You should be able to move off your patches now. Looking forward to catching up soon.

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