metron-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michael Miklavcic <michael.miklav...@gmail.com>
Subject [DISCUSS] Attribution and merging the Elasticsearch client migration
Date Thu, 15 Nov 2018 20:19:21 GMT
https://github.com/apache/metron/pull/1242

TL;DR
I'd like to discuss the best option to merge METRON-1834 into master. I
want to propose handling this like a feature branch and merging it as-is.
---

I'm sure most folks' initial reaction will be some skepticism akin to "have
you tried turning it off again," as this was my initial reaction as well.
It does not seem like this should be difficult. And I'm hoping that this
may be some esoteric thing on my system, though I believe this is a real
problem. A rather tedious explanation follows of what I've tried and the
problems encountered along the way. What seemed like a really simple
problem instead appears to be a bit much for Git to handle without
requiring redoing merges and another full round of testing. I'd much prefer
to avoid that in this instance.

This PR is ready to be merged into master. It's recent and very close to
fully up to date in the branch. Latest master merges cleanly. There is an
attribution to Casey Stella for the base point of this PR that I need to
include when getting this into master. When I created my branch, I
collapsed his initial set of commits into a single squashed commit on
master at the time, and I started to work from there. Over time, I made a
number of additional commits and merges from master. Now for the issues.

Originally, my expectation was that I could have 2 commits - the original
squashed commit from Casey along with all my additional commits (and the
merges with master) right on top. Nice clean history on master. Turns out,
this doesn't work as cleanly as expected because a combination of the
multiple merges and the need to keep the original commit with attribution
to Casey's work. A normal git pull --squash works fine, as expected, but we
lose the base commit, and therefore the requisite attribution. Here are
some other things I've tried, to no avail.

   1. Git pull --squash after a merge with master. This will squash the
   entire tree back to the branch point. No good.
   2. Git rebase -i master. Allows you to cleanly apply changes, but then
   it ends up having problems with a clean rebase and shows conflicts. I
   expect this is because of the merge history being necessary.
   3. Checking out a branch from the base point squashed commit from Casey,
   and attempt to apply my changes on top. Numerous methods for
   squashing/rebasing my changes on top applies nicely in the branch. But then
   it once again causes merge conflicts when I attempt to get this onto
   master. Things I attempted include: manually copying files, rebasing all my
   commits plus merges on top of the base commit, git merge --squash,
   intimidation.

For one example of the result I'm talking about, this looks "good" but it's
missing a ton of recent commits because they get caught up in the rebase
and get squashed in with my commit. When you attempt to merge this onto
master, it is just plain wrong (see example below with merge conflicts).
* 22c3b3bc 2018-11-15 | METRON-1834: Migrate Elasticsearch from
TransportClient to new Java REST API (mmiklavc via mmiklavc) closes
apache/metron#1242 (HEAD -> stella-es-base2) [mmiklavc]
* 84232e90 2018-10-08 | METRON-1834: Elasticsearch rest client migration
base work starting point for apache/metron#1242 (cstella via mmiklavc)
[cstella]
* 5bfc08c5 2018-10-08 | METRON-1792 Simplify Profile Definitions in
Integration Tests (nickwallen) closes apache/metron#1211 [nickwallen]

Here's 1 merge conflict (say what??)
CONFLICT (rename/rename): Rename
"metron-interface/metron-config/src/app/rxjs-operators.ts"->"metron-platform/metron-parsers/src/main/java/org/apache/metron/parsers/ParserRunnerResults.java"
in branch "HEAD" rename
"metron-interface/metron-config/src/app/rxjs-operators.ts"->"metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/utils/FieldMapping.java"
in "stella-es-base2"

If I attempt to use rebase on master instead of merge, it really seems to
mess up the files. Again, another example where I have TODO's that are most
definitely removed by a commit in my branch and also do not exist in
master. I'm not sure what's happening here, but I don't trust it.
    {
      //TODO: It shouldn't require an assertEventually() here as it should
be synchronous.
      // Before merging, please figure out why.
      TestUtils.assertEventually(() -> Assert.assertEquals(14,
dao.getColumnMetadata(Collections.singletonList("snort")).size()));
      Map<String, FieldType> fieldTypes =
dao.getColumnMetadata(Collections.singletonList("snort"));
<<<<<<< HEAD
      Assert.assertEquals(32, fieldTypes.size());
      Assert.assertEquals(FieldType.KEYWORD,
fieldTypes.get("sig_generator"));
=======
      Assert.assertEquals(FieldType.INTEGER, fieldTypes.get("snort_field"));
>>>>>>> METRON-1834: Elasticsearch rest client migration base work starting
point for apache/metron#1242 (cstella via mmiklavc)

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message