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

MODE-456 Create SequencerOutputMap.addProperty Method #146

Closed
wants to merge 1 commit into from

Conversation

bcarothers-xx
Copy link
Member

Added SequencerOutputMap.addPropertyValues(Path, Name, Object...) that adds values to the named property at the given path if it exists, or creates the named property at the given path with the given values if the property did not already exist. This method will cause a PathNotFoundException if no node exists at the given path.

Added new tests to verify behavior and did a quick cleanup on the Getting Started guide where a few paragraphs reviewed the custom security context that no longer exists in the sequencer example. All tests pass.

Added SequencerOutputMap.addPropertyValues(Path, Name, Object...) that adds values to the named property at the given path if it exists, or creates the named property at the given path with the given values if the property did not already exist.  This method will cause a PathNotFoundException if no node exists at the given path.

Added new tests to verify behavior and did a quick cleanup on the Getting Started guide where a few paragraphs reviewed the custom security context that no longer exists in the sequencer example.  All tests pass.
@rhauch
Copy link
Contributor

rhauch commented Jul 18, 2011

I'm getting an error in shouldFindVdbsUsingQueryWithMultipleVariables(org.modeshape.test.integration.sequencer.TeiidSequencerIntegrationTest) every time I run an integration build using these changes.

@rhauch
Copy link
Contributor

rhauch commented Aug 8, 2011

@bcarothers, I'm still getting the error I mention above, even after I apply your changes to the very latest on 'master'. Note that the failure is because of the test attempting to query the derived content produced by the sequencer. Since this pull-request changes how the derived content is produced/written, I'm concerned the changes are not completely correct.

@bcarothers-xx
Copy link
Member Author

I'm getting the same thing locally. I agree with your assessment that the fix must not be correct. I haven't had a lot of time to look into the problem though and have no familiarity with the query engine, so this will not be something that I am likely to fix anytime soon. Please feel free to reject the change and close the pull request if that's better than leaving it open until I get this fixed/cleaned up. Thanks

@rhauch
Copy link
Contributor

rhauch commented Aug 8, 2011

@bcarothers I just merged a change that fixes the query error so that the test case fails by simply reporting the wrong number of expected results. IOW, the generated content is simply not what the queries expect to find, so you shouldn't have to worry about the query engine. Just be sure to rebase your local branch off the latest from 'master'.

@rhauch
Copy link
Contributor

rhauch commented Oct 27, 2011

Closing WITHOUT merging, due to the aforementioned issues.

@rhauch rhauch closed this Oct 27, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants