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

Fixes #652: Allow amps to be updated and not just created #614

Merged
merged 2 commits into from
May 25, 2016

Conversation

ghost
Copy link

@ghost ghost commented May 24, 2016

Previously, existing amp definitions were not changed by bootstrap, now the roles are updated when bootstrapping. #652

…p definitions were not changed by bootstrap).
<database>{xdmp:security-database()}</database>
</options>
),
setup:add-rollback("amps", $amp)
Copy link
Collaborator

Choose a reason for hiding this comment

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

An error condition will result in the amp getting wiped out during rollback, rather than restored to its original roles. I think it's better to not call setup:add-rollback() here, analogous to setup:apply-database-settings().

@ghost
Copy link
Author

ghost commented May 25, 2016

OK, I updated the PR based on that suggestion. Totally makes sense, thanks for the review.

@grtjn
Copy link
Contributor

grtjn commented May 25, 2016

Hmm, just wondering: can you specify multiple roles for an amp? We usually tend to add a wrapping element if multiple would be allowed. I think it would work, but just pondering we lack the wrapper. Makes me think whether the built-in format would use that or not, as that is probably what would be returned by capture (though not sure capture returns amps)..

@dmcassel
Copy link
Collaborator

sec:amp-set-roles() does take $role-names as xs:string*. The submitted code is consistent with how amps are created, so I think we're okay there.

@dmcassel dmcassel merged commit 5baaaab into marklogic-community:dev May 25, 2016
@dmcassel
Copy link
Collaborator

@tapuzzo-fsi thanks for the PR!

@ghost
Copy link
Author

ghost commented May 25, 2016

Yes, I've tested this with multiple roles and it works as expected. Thanks for accepting the PR.

@ghost ghost deleted the dev branch May 25, 2016 20:18
@ghost ghost restored the dev branch May 25, 2016 20:18
@ghost ghost deleted the dev branch May 26, 2016 22:11
@grtjn grtjn changed the title Allow amps to be updated and not just created #652: Allow amps to be updated and not just created Aug 23, 2016
@grtjn grtjn changed the title #652: Allow amps to be updated and not just created Fixes #652: Allow amps to be updated and not just created Aug 23, 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.

2 participants