Skip to content

feature parity with legacy ytproxy#18

Open
francescayeye wants to merge 9 commits intoTeamPiped:mainfrom
francescayeye:feature-parity-with-ytproxy
Open

feature parity with legacy ytproxy#18
francescayeye wants to merge 9 commits intoTeamPiped:mainfrom
francescayeye:feature-parity-with-ytproxy

Conversation

@francescayeye
Copy link
Copy Markdown

  • legacy ytproxy accepts a PREFIX_PATH env variable for special deployment case where the proxy is hosted behind a path rather on a separate domain
  • legacy ytproxy filters out more sensitive headers to be forwarded

@francescayeye
Copy link
Copy Markdown
Author

@FireMasterK , I was rewriting the response in nginx with lua, to have the urls from proxy be prepended by the proxy path. but it was very slow and produced a lot of buffering, so having support for it directly in the proxy would be awesome.

please, let me know what do you think of this feature

thanks :)

Copy link
Copy Markdown
Member

@FireMasterK FireMasterK left a comment

Choose a reason for hiding this comment

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

I want this to be a configurable non-default feature at build time.

@francescayeye
Copy link
Copy Markdown
Author

I want this to be a configurable non-default feature at build time.

will you plan to release a separated docker image with that build?

@FireMasterK
Copy link
Copy Markdown
Member

will you plan to release a separated docker image with that build?

There could be an image with all build features.

@francescayeye
Copy link
Copy Markdown
Author

@FireMasterK fixed, pleae let me know if anything else is missing

if qhash.is_some() {
let mut query = QString::new(query.into_iter().collect::<Vec<_>>());
query.add_pair(("qhash", qhash.unwrap()));
return format!("{}?{}", path, query);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We need feature specific code for when prefix-path enabled and not.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

hi @FireMasterK , please let me know if my solution is what you meant. thanks

@francescayeye
Copy link
Copy Markdown
Author

glad to go back to this PR and finally have it merged to master, maybe it will be the chance for me to go back to use piped :D

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.

2 participants