These are chat archives for atomix/atomix

30th
May 2018
Ronnie
@rroller
May 30 2018 00:06
The unit tests pass but the service fails to start with the above error when running on my desktop and production
Ronnie
@rroller
May 30 2018 00:44
which is odd becuase otherwise it's the same code
Ronnie
@rroller
May 30 2018 02:40
this is the code that throws the exception
      PartitionGroupType partitionGroupType = config.getRegistry().getPartitionGroupTypes().get(partitionGroupConfig.getType());
      if (partitionGroupType == null) {
        throw new ConfigurationException("Unknown partition group type " + partitionGroupConfig.getType());
      }
I set a break point and went into debug mode. config.getRegistry().getPartitionGroupTypes() is empty.
is there something new that's required?
Ronnie
@rroller
May 30 2018 03:31
still unexplainable.
builder.withPartitionGroups(RaftPartitionGroup.builder("raft")
                .withNumPartitions(1)
                .withMembers(members)
                .withDataDirectory(dataDir)
                .build());
on an Atomix.builder()... doesn't seem to register the type, though it does with the same code in my unit tests.
that code path to that point is rather small. will step through it more
Johno Crawford
@johnou
May 30 2018 07:14
@rroller how are you packaging the application
Would be easier to debug if you provided a complete reproducer project
Jordan Halterman
@kuujo
May 30 2018 09:17
strange indeed
are you using the shade plugin by chance?
because the changes split the registry into multiple files I had to merge them together in an uber jar
Johno Crawford
@johnou
May 30 2018 09:39
yeah that's where I was going with packaging :)
that does the trick
Ronnie
@rroller
May 30 2018 13:48
Yeha I’m producing an Uber jar
Are you suggesting I need to do that same as the code above in the link you pasted?
Ronnie
@rroller
May 30 2018 14:32
I pulled down the latest snapshot versions that include the change above, didn't help
Johno Crawford
@johnou
May 30 2018 14:57
@rroller you'll need to modify your uber jar configuration to use the AppendingTransformer like the agent packaging config
Ronnie
@rroller
May 30 2018 15:00
I see. That's very difficult in my case
Seems like that's a deal breaker for a lot of people IMO
What's the content of that file? Can I construct the registry directly in java?
Ronnie
@rroller
May 30 2018 15:12
Ok I added the AppendingTransformerand it worked. The issue with this though is that my org uses a parent pom that has the shade plugin for us. So for me to add the AppendingTransformer I had to stop using the parent pom and copy its contents into my service directly so that I could add the AppendingTransformer. That's going to be hard for me to convince other teams here to do
Johno Crawford
@johnou
May 30 2018 17:16
why?
just override the config
leave the org parent pom, then re declare the shade plugin (you can omit the version) and add the transformer tags
Ronnie
@rroller
May 30 2018 17:19
yeah, that's doable. But it doesn't seem ideal to even need to do this at all
Are there not other options to restructure Atomix so that users of the library don't need to do this at all? Prior to these change it wasn't needed and it made onboarding much easier... I think this will be a pain point for users of the library
Johno Crawford
@johnou
May 30 2018 17:20
if that's how you ship your application it should be expected, that's a downside of shading, same goes for spring deps
the point of structuring it that way is so it is modular and you can decide on what to include
Ronnie
@rroller
May 30 2018 17:24
Can you link me to the actualy conf file. I can't find it in the repo
Johno Crawford
@johnou
May 30 2018 17:25
it's not in master, it's in the typesafe branch
master still uses service loader
Ronnie
@rroller
May 30 2018 17:26
It looks like there's a core/src/main/resources/defaults.conf, though empty. So I should be including that as well
and we'll need to keep up with any future conf files added
Ronnie
@rroller
May 30 2018 17:55
btw, the override does work better (rather than copying the entire parent pom, just add the specific append config)
And I think I missed the point earlier. Is the point of doing this because there's two registry.conf files? So they were not included because of conflict? So isntead we just append with the transformer?
Jordan Halterman
@kuujo
May 30 2018 18:10
yeah
There are three of them: one in the core, one in atomix-primary-backup and atomix-raft
Jordan Halterman
@kuujo
May 30 2018 18:41
The alternative would be to resolve the classes at startup and ignore classes not found on the classpath but that can cause some much less obvious bugs
Should probably just create more helpful error messages
Jordan Halterman
@kuujo
May 30 2018 18:48
I got Atomix working in ONOS… just cleaning it up and will be finishing up Atomix for the RC as I go along
Ronnie
@rroller
May 30 2018 18:57
Is it possible to give them different names? Do they all need the same name?
Jordan Halterman
@kuujo
May 30 2018 18:58
the core can’t know a name it doesn’t know, so yes
it’s for extensibility, and the core needs to know the file it’s looking for otherwise we’d have to hardcode a set of file names which would limit the extensibility
Ronnie
@rroller
May 30 2018 19:00
Extensible by who? Those working on Atomix or end users like me? Both?
Jordan Halterman
@kuujo
May 30 2018 19:00
both
it’s how you add new primitive types
Jordan Halterman
@kuujo
May 30 2018 19:18
BTW this is the same thing that has to be done with ServiceLoader for the same reason
Ronnie
@rroller
May 30 2018 20:39
I wonder why I didn't hit that before
Jordan Halterman
@kuujo
May 30 2018 20:40
just recently made those dependencies optional
Jordan Halterman
@kuujo
May 30 2018 21:18
Actually that doesn’t make sense. I have no idea ¯(ツ)
Gitter doesn’t like by backslashes
Ronnie
@rroller
May 30 2018 21:22
¯\(ツ)/¯
At some point I'd like to really dig into this and understand this more and see what we can do to avoid needing to touch the poms. I think this is going to cause lots of pain in the future.
especially when tests will pass because we aren't using an uber jar for tests, but then rolling out to prod things fail
not a blocker now though