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

Add hook for instance startup #1888

Merged
merged 1 commit into from
Oct 2, 2017
Merged

Conversation

tantalic
Copy link

1. What does this change do, exactly?

Provides a new hook that plugins can use to gain access to the caddy.Instance. This is particularly important to allow plugins the ability to dynamically generate a new Caddyfile and reload the server with the new configuration.

With this change in place a plugin could access the instance as follows:

func init() {
	caddy.RegisterEventHook("sampleinithook", SampleInitHook)
}

func SampleInitHook(eventType caddy.EventName, eventInfo interface{}) error {
	if eventType != caddy.InstanceStartupEvent {
		return nil
	}

	instance, ok := eventInfo.(*caddy.Instance)
	if !ok {
		return errors.New("Invalid eventInfo provided for caddy.InstanceStartupEvent event")
	}

	// save the instance for later use and/or do something useful with it here
}

2. Please link to the relevant issues.

3. Which documentation changes (if any) need to be made because of this PR?

4. Checklist

  • I have written tests and verified that they fail without my change - I have verified existing tests pass but it appears the events are not currently included in tests and I see no obvious way to add tests for this case
  • I have squashed any insignificant commits
  • This change has comments for package types, values, functions, and non-obvious lines of code
  • I am willing to help maintain this change if there are issues with it later

@CLAassistant
Copy link

CLAassistant commented Sep 20, 2017

CLA assistant check
All committers have signed the CLA.

@elcore elcore added the duplicate 🖇️ This issue or pull request already exists label Sep 20, 2017
@elcore
Copy link
Collaborator

elcore commented Sep 20, 2017

Hello @tantalic,

I am going to close this PR as I already implemented it (Reference : #1879)

@elcore elcore closed this Sep 20, 2017
@@ -199,6 +199,9 @@ func (i *Instance) Restart(newCaddyfile Input) (*Instance, error) {
}
i.Stop()

// Execute instantiation events
EmitEvent(InstanceStartupEvent, newInst)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I will add this to my PR 😄

@mholt mholt removed the duplicate 🖇️ This issue or pull request already exists label Sep 20, 2017
@mholt
Copy link
Member

mholt commented Sep 20, 2017

Hey Eldin, I think it would be fair to make sure that @tantalic gets the credit in the commit; it's OK if there are two separate PRs. Smaller PRs are better. 😉

@mholt mholt reopened this Sep 20, 2017
@elcore
Copy link
Collaborator

elcore commented Sep 20, 2017

Hey Eldin, I think it would be fair to make sure that @tantalic gets the credit in the commit; it's OK if there are two separate PRs. Smaller PRs are better. 😉

I have no issues giving credit for the first part of this PR ☺️ - - It looks like this PR is based upon my changes. (Reference : elcore@7056893)

You can merge this PR. I'll update my PR....

@elcore elcore mentioned this pull request Sep 27, 2017
4 tasks
@mholt
Copy link
Member

mholt commented Oct 1, 2017

@tantalic Can you please rebase with master? I'll begin review after the merge conflict is resolved.

Provides a new hook for plugins as a means to provide the current caddy.Instance when starting or restarting.
@tantalic
Copy link
Author

tantalic commented Oct 2, 2017

@mholt - The changes are now rebased on master. It appears CI builds are failing for some reason outside the scope of this change now. Please advise if you need anything else on this PR.

@mholt mholt merged commit 97710ce into caddyserver:master Oct 2, 2017
@mholt
Copy link
Member

mholt commented Oct 2, 2017

Thanks for contributing! Really appreciate it. This'll go out in the next release.

@mholt mholt added this to the 0.10.10 milestone Oct 9, 2017
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.

4 participants