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

Endpoints urls with slash #3824

Merged
merged 4 commits into from
Jun 20, 2019
Merged

Endpoints urls with slash #3824

merged 4 commits into from
Jun 20, 2019

Conversation

wochinge
Copy link
Contributor

@wochinge wochinge commented Jun 19, 2019

Proposed changes:

Status (please check what you already did):

  • made PR ready for code review
  • added some tests for the functionality
  • updated the documentation
  • updated the changelog
  • reformat files using black (please check Readme for instructions)

"target server supports trailing slashes for this "
"endpoint.".format(base)
)
return base

url = base
if not base.endswith("/"):
Copy link
Contributor

@erohmensing erohmensing Jun 19, 2019

Choose a reason for hiding this comment

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

@wochinge I'm not exactly sure what's going on here -- above, we throw a debug message(and previously stripped the slash) if there was one, and then below we add one back on?

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 decided against, cause if subpath values are added by Rasa code (and not externally) and we should know what we are doing and if we add a slash or not.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah okay I got it, I was confused bc i was missing that these were 2 cases with and w/o subpath

Copy link
Contributor

@erohmensing erohmensing left a comment

Choose a reason for hiding this comment

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

Understand now, looks good 👍

"target server supports trailing slashes for this "
"endpoint.".format(base)
)
return base

url = base
if not base.endswith("/"):
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah okay I got it, I was confused bc i was missing that these were 2 cases with and w/o subpath

@erohmensing erohmensing merged commit 8c04bae into master Jun 20, 2019
@erohmensing erohmensing deleted the endpoints_urls_with_slash branch June 20, 2019 12:09
@Wing-e7
Copy link

Wing-e7 commented Apr 10, 2020

Still doesnt solve the 404 issue. Please help

@erohmensing
Copy link
Contributor

@Wing-e7 please open a bug report with full information and ideally reproducible behavior if you think there is still a bug!

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.

Removing trailing slash results in weird behavior
3 participants