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

Removing trailing slash results in weird behavior #3776

Closed
1 task done
wenig opened this issue Jun 13, 2019 · 2 comments · Fixed by #3824
Closed
1 task done

Removing trailing slash results in weird behavior #3776

wenig opened this issue Jun 13, 2019 · 2 comments · Fixed by #3824
Assignees
Labels
area:rasa-oss 🎡 Anything related to the open source Rasa framework type:bug 🐛 Inconsistencies or issues which will cause an issue or problem for users or implementors.

Comments

@wenig
Copy link

wenig commented Jun 13, 2019

Description of Problem:
That change 7fbc08b#diff-c66e6bc1dfd15c342f53b163376f7270R48 results in weird behavior. If I define an url in endpoints.yml (for example the nlg endpoint) which is required to end with a slash, the slash is removed and a request without a slash is sent. This shouldn't be a problem as my nlg server redirects this request to the path with a slash. However, the library aiohttp which is used to request has the following behavior aio-libs/aiohttp#3082. It will change the method from POST to GET after a redirect. This results in a "405 Method not allowed".

Overview of the Solution:
Of course, I can adapt my nlg server to not redirect and directly take the path without slash but it is still not right in my opinion to change the urls which are defined in some config files. I'd rather have the obvious behavior and requests to the urls I defined.

Definition of Done:

  • return base.rstrip("/") --> return base
@wenig wenig added the type:enhancement ✨ Additions of new features or changes to existing ones, should be doable in a single PR label Jun 13, 2019
@akelad
Copy link
Contributor

akelad commented Jun 13, 2019

this sounds more like a bug report than an enhancement right?

@wochinge since you merged that PR, can you take a look at it?

@tmbo tmbo added type:bug 🐛 Inconsistencies or issues which will cause an issue or problem for users or implementors. and removed type:enhancement ✨ Additions of new features or changes to existing ones, should be doable in a single PR labels Jun 14, 2019
@tmbo tmbo added the area:rasa-oss 🎡 Anything related to the open source Rasa framework label Jun 17, 2019
@wochinge
Copy link
Contributor

@wenig Thanks for this detailed bug report! I'm reverting the stripping here and replacing it with a debug message: #3824

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:rasa-oss 🎡 Anything related to the open source Rasa framework type:bug 🐛 Inconsistencies or issues which will cause an issue or problem for users or implementors.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants