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

[WIP] Add Varnishadm CLI client #231

Open
wants to merge 1 commit into
base: 3.x
Choose a base branch
from
Open

[WIP] Add Varnishadm CLI client #231

wants to merge 1 commit into from

Conversation

ddeboer
Copy link
Member

@ddeboer ddeboer commented Jul 5, 2015

Fix #61.

  1. The idea is that using this client involves less custom VCL. However, for the ban lurker to work, some custom VCL is still necessary to copy e.g. the URL header to X-Url.
  2. As far as I know, purge cannot be done through varnishadm, so we should either not implement PurgeInterface or fake the purge by actually doing a ban when purge() is called.
  3. Same goes for refresh. We could fake it by doing a ban and then manually getting a fresh response from the application.

{

}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure yet whether we should support multiple servers in the VarnishAdmin class itself, or wrap them in a separate class such as this VarnishAdminMultiple.

Copy link
Contributor

Choose a reason for hiding this comment

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

looking at VarnishAdmin, i wonder if we should have a VarnishCli class that wraps the socket connection. then VarnishAdmin would become a lot simpler, and having multiple Cli instances would become simpler. if we keep it the way we currently have it, having a separate wrapper to multiplex seems better to me.

@ddeboer ddeboer added this to the 2.0 milestone Jul 5, 2015
@dbu
Copy link
Contributor

dbu commented Jul 6, 2015

very cool! my main question at this point is whether we could move the code dealing with sockets and sprintf-ing commands into a VarnishCli class to better separate the concerns.

@ddeboer
Copy link
Member Author

ddeboer commented Jul 6, 2015

We can add a VarnishCli class, and it will help in separating concerns a bit, but I don’t think we’ll be communicating with any other Varnish command line tools.

Do you mean a VarnishAdmin that wraps multiple VarnishClis for talking to multiple servers?

@dbu
Copy link
Contributor

dbu commented Jul 7, 2015 via email

@dbu
Copy link
Contributor

dbu commented Jan 26, 2016

rebased to get rid of the outdated php-http reference

@dbu
Copy link
Contributor

dbu commented Mar 30, 2016

@ddeboer any chance you could wrap this up for 2.0?

@ddeboer
Copy link
Member Author

ddeboer commented Mar 31, 2016

As this is no priority for myself now, I suggest to remove this from the 2.0 milestone so we can move forward on 2.0. I we can add this functionality on the 2.0 branch later on, that’s fine. If we need to break BC it can wait until 3.0 (for me).

@dbu dbu removed this from the 2.0 milestone Mar 31, 2016
@dbu
Copy link
Contributor

dbu commented Mar 31, 2016

pitty, but i removed the milestone

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.

None yet

2 participants