Skip to content
This repository has been archived by the owner on Nov 9, 2022. It is now read-only.

Issue 109: Adding support for deploy/clean triggers #577

Merged
merged 16 commits into from
Apr 4, 2016

Conversation

dmcassel
Copy link
Collaborator

@dmcassel dmcassel commented Mar 4, 2016

Added commands:

  • ml {env} deploy triggers
  • ml {env} clean triggers

The code tests to make sure you have a Triggers database configured. The clean wipes out all triggers in the target database, so users are advised to have an application-specific triggers database. #109

1. Copy deploy/sample/triggers-config.sample.xml to deploy/triggers-config.xml
2. Edit deploy/triggers-config.xml to specify your trigger(s)
3. Run 'ml <env> deploy triggers')
ERR
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice error handling..

@grtjn
Copy link
Contributor

grtjn commented Mar 8, 2016

Tidy work! Above a few inline comments..

@dmcassel
Copy link
Collaborator Author

The only outstanding comment is about self-testing. @grtjn, what do you say we merge this PR and then address testing separately? (Ideally it would all be together, but I'd like to get this integrated and move on.)

@grtjn
Copy link
Contributor

grtjn commented Mar 10, 2016

Understood. Integrating this into self-test now makes it easier for all to test this, though. I only looked at the code so far, haven't ran it yet.

I had a quick thought on this, and was thinking you might get away with it by adding to extra test steps to the bootstrap test case. Somewhere around here:

https://github.com/marklogic/roxy/blob/master/deploy/test/test_server_config.rb#L101

Copy the trigger config into test/data/, point to it in the step, and run deploy_triggers. A quick count on trigger docs in the triggers db could verify success/fail. the second step would run clean_triggers, and the count should return zero. Nothing too fancy, just make sure the code runs without errors, and something gets created/cleaned..

Doable?

@grtjn
Copy link
Contributor

grtjn commented Mar 18, 2016

Self-test doesn't run out of the box. I think some lookup fails if you don't run against a clustered environment. It is relatively easy to make it work though, just edit this line:

https://github.com/marklogic/roxy/blob/dev/deploy/lib/xquery/setup.xqy#L1108

Replace $hosts[1] with ($hosts, $default-host)[1].

You may also want to comment this line:

https://github.com/marklogic/roxy/blob/dev/deploy/lib/xquery/setup.xqy#L5589

It produces a lot of unnecessary comments in the ErrorLog..

With these two changes the self-test on dev branch runs clean. The only odd thing is that the credentials test doesn't seem to work automatically anymore. I need to manually type bob/smith..

<trgr:depth>1</trgr:depth>
</trgr:directory-scope>
<trgr:document-content>
<trgr:update-kind>modify</trgr:update-kind>
Copy link
Contributor

Choose a reason for hiding this comment

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

CreateTrigger but update-kind modify?

@dmcassel
Copy link
Collaborator Author

Okay, I added triggers to self-test. Doesn't do a whole lot of validation at this point, but it does run it, so big errors should get caught.

@s.deploy_triggers.must_equal true
@s.clean_triggers.must_equal true
end

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a triggers-config.xml, or is the self-test running with the sample one?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This ran for me because I already had a triggers-config file in deploy. Just revised so that it used the one in the sample directory for the test.

@grtjn
Copy link
Contributor

grtjn commented Mar 22, 2016

FYI, applied this for a project, deployment of triggers worked like a charm, thanks!

Though, could you get rid of this xdmp:log statement: https://github.com/dmcassel/roxy/blob/issue-109/deploy/lib/xquery/setup.xqy#L5507. It's debug info..

@dmcassel
Copy link
Collaborator Author

@grtjn have I addressed all your concerns?

@grtjn
Copy link
Contributor

grtjn commented Mar 23, 2016

I only notice one more thing:

You fixed the trigger config for ml7 self-test, but not for ml6, despite the fact you did enable triggers-db in ml6..

If you can attend to that, I'll try to run some elaborate self-tests tomorrow morning..

@dmcassel
Copy link
Collaborator Author

Oops, missed ml6. Just added that.

@dmcassel
Copy link
Collaborator Author

@grtjn have you had a chance to run tests?

@grtjn
Copy link
Contributor

grtjn commented Mar 31, 2016

Ran a self-test, but the deploy triggers is failing:

I, [2016-03-31T10:39:20.746472 #36838]  INFO -- : Deploying Triggers
E, [2016-03-31T10:39:20.752485 #36838] ERROR -- : ml.triggers.file=/Users/gjosten/Projects/github-grtjn/roxy/deploy/triggers-config.xml
E, [2016-03-31T10:39:20.752592 #36838] ERROR -- : Before you can deploy triggers, you must define a configuration. Steps:
1. Copy deploy/sample/triggers-config.sample.xml to /Users/gjosten/Projects/github-grtjn/roxy/deploy/triggers-config.xml
  The location of this file is controlled by the triggers.file property.
2. Edit /Users/gjosten/Projects/github-grtjn/roxy/deploy/triggers-config.xml to specify your trigger(s)
3. Run 'ml <env> deploy triggers')

Weird though since properties and all are looking correct. Need to take a closer look later..

@grtjn
Copy link
Contributor

grtjn commented Apr 4, 2016

We decided to move selftest changes out of this PR. It is ready to go..

@grtjn grtjn merged commit 045c3ce into marklogic-community:dev Apr 4, 2016
@dmcassel dmcassel deleted the issue-109 branch April 14, 2016 20:52
@grtjn grtjn changed the title Issue 109 Issue 109: Adding support for deploy/clean triggers Aug 22, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants