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

Solve the --path_prefix problem #1620

Closed

Conversation

zhangchen-qinyinghua
Copy link

Fix a bug on implementing the --path_prefix.
The bug happens because all of the keys in the structure self.data_applications has path_prefix as their prefix.
But the function _clean_path just removes the prefix by mistake.

Fix a bug on implementing the ```--path_prefix```.
The bug happens because all of the keys in the structure ```self.data_applications```  has ```path_prefix``` as their prefix.
But the function ```_clean_path``` just removes the prefix by mistake.
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

@wchargin
Copy link
Contributor

Hi @zhangchen-qinyinghua! Thanks for the contribution.

You’re right that the --path_prefix argument is totally broken
currently. Thanks for bringing this to our attention.

I’m not sure that it’s related to the issue that you referenced (#1176);
that issue is just a display problem that existed before the current
problems with --path_prefix were introduced (in #1512).

The code in this PR is not the right approach to solving the
--path_prefix problem. If --path_prefix is set to /test/, then
clients should be able to fetch routes like /test/data/runs. The
problem is that the frontend is currently requesting routes like
/data/runs, without the prefix. The fix is to change the frontend to
request the right routes, not to change the backend to effectively
ignore the path prefix altogether: with this change, I can fetch
/data/runs but not /test/data/runs, so --path_prefix effectively
only applies to the root route.

Let me know if this doesn’t make sense to you or you think that this
analysis is in error. Thanks again!

cc @stephanwlee

@wchargin wchargin closed this Nov 21, 2018
@zhangchen-qinyinghua
Copy link
Author

Yeah, sure.
I noticed that my fixing is just kind of hacking without solving the problem essentially.
This PR seems to fix it in the right way.

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.

3 participants