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

Fix/deferred location config #1093

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

alasdairhodge
Copy link
Contributor

Resolves issues with $brooklyn:external() in location config:

  • TypeCoercions resolves DeferredSupplier values
  • locations constructed within a new execution context on LocalLocationManager so that relevant context objects can be retrieved in a consistent way

@alasdairhodge
Copy link
Contributor Author

Once this is reviewed/merged into master, I'll issue the same fix against 0.8.x.

locationTypes = storage.getMap("locations");
this.locationTypes = storage.getMap("locations");

BasicExecutionManager executionManager = new BasicExecutionManager(managementContext.getManagementNodeId());
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really need to create our own BasicExecutionManager? Can we not reuse managementContext.getExecutionManager()?

@aledsage
Copy link
Contributor

@alasdairhodge Looks good. Only one important comment about reusing the ManagementContexts ExecutionManager, rather than creating a new one.

@aledsage
Copy link
Contributor

Actually, can we have a test that externalised configuration now works in locations? As I recall, it was only in one situation that it failed - when the config was accessed during the init of the location object? For example in JcloudsLocation.setCreationString(), which is called by JcloudsLocation.configure()?

@aledsage
Copy link
Contributor

How about a test like this in ExternalConfigYamlTest:

@Test
public void testExternalisedLocationConfigAccessedDuringLocationInit() throws Exception {
    String yaml = Joiner.on("\n").join(
        "services:",
        "- type: org.apache.brooklyn.core.test.entity.TestApplication",
        "location:",
        "  aws-ec2:",
        "    identity: myidentity",
        "    credential: mycredential",
        "    region: $brooklyn:external(\"myprovider\", \"mykey\")");

    TestApplication app = (TestApplication) createAndStartApplication(new StringReader(yaml));
    waitForApplicationTasks(app);
    LocationInternal loc = (LocationInternal) Iterables.getOnlyElement( app.getLocations() );
    assertEquals(loc.config().getLocalBag().getDescription(), "aws-ec2:myHardcodedVal");
}

Does that now pass for you? It fails for me in master:

org.apache.brooklyn.util.exceptions.CompoundRuntimeException: Unable to instantiate item, 2 errors including: Transformer for brooklyn-camp gave an error creating this plan: Illegal parameter for 'location' (aws-ec2); not resolvable: java.lang.ClassCastException: org.apache.brooklyn.camp.brooklyn.spi.dsl.methods.BrooklynDslCommon$DslExternal cannot be cast to java.lang.String

@aledsage
Copy link
Contributor

@alasdairhodge any progress on including that test and getting it to pass? Ping me if you want to discuss or pair.

@ahgittin
Copy link
Contributor

@alasdairhodge ping

1 similar comment
@ahgittin
Copy link
Contributor

@alasdairhodge ping

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.

3 participants