zipkin-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Adrian Cole <adrian.f.c...@gmail.com>
Subject Re: [VOTE] Release Apache Zipkin Brave Karaf (incubating) version 0.1.2
Date Mon, 11 Feb 2019 00:22:23 GMT
TL;DR; ./mvnw test is probably all we need to do in order to verify
the release from a process POV (same as Dubbo). I don't think that
warrants a special additional file. In fact the reason I re-did the
release last night was to make sure it wouldn't :P


The reason I'm hesitant on adding another instruction file is that "no
good deed goes unpunished". What I mean by this is that promoting that
this is the official source does not imply we should lead people into
using that directly. I'm concerned that adding an additional file,
something that isn't commonly included at least in projects I've been
involved with, will set a precedent of yet another support burden. For
example.. I tried to run your build to do X and Y happened. Then, we
get to figure out if someone monkeyed with the code or not.

Instead, I think we should promote the official binary releases and
those things derived from them (ex in the case here there is nothing,
but in zipkin itself, the server would be a great example). Having a
special instruction file and only referencing how to build from
source, in other words.. seems destined to land on others, maybe
people not in this mail thread yet (future maintainers). That's why I
prefer (even if we weren't talking about brave-karaf) to leave it out.

When I mentioned the commands that Zoltan used, I was mentioning them
for completeness. I don't think people in normal circumstances will
want to download our source dist for the sole purpose of building the
source dist from within it :P This is a verification step I'm not even
sure other apache projects do. It isn't listed on the few READMEs I've
seen.

Does this make sense?

-A

On Mon, Feb 11, 2019 at 12:30 AM Adrian Cole <adrian.f.cole@gmail.com> wrote:
>
> Do you know an example of any apache project that has a second variant of a README for
this purpose?
>
> I still think we shouldn't be adding more steps than others are doing. There is absolutely
nothing unique about using maven compile or package goal. it is default.
>
> On Sun, Feb 10, 2019, 10:46 PM Tommy Ludwig <tommy.ludwig.csjf@gmail.com wrote:
>>
>> I think it would be best if we have a README file in the source that
>> includes instructions on how to build. Someone downloading the source
>> release should ideally be able to learn how to build that release from the
>> included sources, IMO. At least this is my interpretation of the Apache
>> way. It would ensure that if the instructions for how to build the project
>> change, we don't need to keep a history separately (e.g. in a Wiki) since
>> the instructions included in the release should always be correct for that
>> release and would be tested by the PMC during release voting.
>>
>> On Sun, Feb 10, 2019 at 11:09 PM Adrian Cole <adrian.f.cole@gmail.com>
>> wrote:
>>
>> > OK all: the new git SHA is 31545805a55dbe5e495403d84172fc865a4935e0
>> >
>> > Lesson learned is that we should be specific about how precisely to
>> > test things, and that RMs (me in this case) should attempt those
>> > things in a completely clean env (ex nuking all the caches).
>> >
>> > Towards that end, beyond the usual apache stuff. If you want to test
>> > the contents themselves, execute basically the same stuff as dubbo
>> > mentions in their "Verify Release Candidates" section. We have the
>> > same requirements as they do, but they already have a TODO list. We
>> > can make one like this later I guess.
>> >
>> >
>> > https://dubbo.incubator.apache.org/en-us/blog/prepare-an-apache-release.html
>> >
>> > I don't know if we restart the 3 days now or not.. it doesn't really
>> > matter as no-one is blocking on this 0.1 release. Anyway, appreciate
>> > in advance those who are up to practicing verifying a release!
>> > -A
>> >
>> > On Sun, Feb 10, 2019 at 2:13 PM Adrian Cole <adrian.f.cole@gmail.com>
>> > wrote:
>> > >
>> > > Thanks for understanding, Zoltan. FWIW, the test in question had the
>> > > incorrect naming convention for an integration test. Tommy noticed
>> > > that it should have never run in the package phase anyway. This
>> > > corrects that:
>> > https://github.com/apache/incubator-zipkin-brave-karaf/pull/28
>> > >
>> > > On Sun, Feb 10, 2019 at 1:35 PM Zoltán Nagy <abesto@apache.org>
wrote:
>> > > >
>> > > > Vote: +1
>> > > >
>> > > > Yeah in general I'd say trust that CI has already verified we're OK,
>> > and
>> > > > don't run integration tests on package / install in the released
>> > artifacts.
>> > > > Good point about Docker as well. I've just tried `./mvnw package`
on a
>> > > > clone from git - first run passed, then the second one failed with
the
>> > same
>> > > > error as the source release. I'm ready to let this go as "finicky
>> > > > integration tests".
>> > > >
>> > > > The release is Good Enough (TM) as is IMHO, assuming we provide some
>> > > > command-line to package it up without running the integration tests.
Of
>> > > > course it'd be even nicer if users didn't even have to do _that_,
but
>> > let's
>> > > > not block the release on this.
>> > > >
>> > > > Note on stripping itests from the source release: ideally that'd be
>> > done
>> > > > without modifying the code-base, since one of the steps pre-vote is
>> > > > verifying that the code in the release is actually the code in the
>> > cited
>> > > > commit hash - release-time code modification makes that harder (though
>> > not
>> > > > impossible, the difference should be trivial in any case). I _guess_
>> > that'd
>> > > > mean making package / install / whatever not run itests, and adding
>> > itests
>> > > > to CI explicitly, but as it's been proven several times, I only have
>> > > > superficial knowledge of Maven, so I'll stop trying to guess here.
>> > > >
>> > > > On Sun, Feb 10, 2019 at 12:13 PM Adrian Cole <adrian.f.cole@gmail.com>
>> > > > wrote:
>> > > >
>> > > > > FWIW the build worked on my laptop. it might be an environment
>> > nuance.
>> > > > > Karaf itests are sensitive, and so for example end users shouldn't
>> > > > > require anything delicate. For example, would you fail a build
>> > because
>> > > > > a user can't install docker that's a prereq? We are only vetting
code
>> > > > > I think we should step back from signing ourselves up to make
every
>> > > > > potential user able to run all the myriad of integration tests
>> > > > > possible in our environments.
>> > > > >
>> > > > > On Sun, Feb 10, 2019 at 1:09 PM Zoltán Nagy <abesto@apache.org>
>> > wrote:
>> > > > > >
>> > > > > > > however if there is something in the zip it should
work.
>> > > > > >
>> > > > > > Agree. I pretended to be a user with relatively little knowledge
>> > about
>> > > > > the
>> > > > > > internals here - with that hat on, I don't mind whether
it's the
>> > itests
>> > > > > or
>> > > > > > whatever else that's failing, all I know is that `./mvnw
package`
>> > should
>> > > > > > give me something I can deploy.
>> > > > > >
>> > > > > > > I will lightly look into how much logic we need in
>> > > > > > > general to strip itests out completely.
>> > > > > >
>> > > > > > Weak opinion: I like that tests are shipped with the release,
so
>> > that
>> > > > > (if I
>> > > > > > use the source release) there's a last line of defense against
>> > badness.
>> > > > > Not
>> > > > > > to mention, this way tests are also run in an environment
that's
>> > closer
>> > > > > to
>> > > > > > my production. Still, this is mostly academical - I expect
99% of
>> > our
>> > > > > users
>> > > > > > to use the binary convenience artifacts, so the point is
somewhat
>> > moot.
>> > > > > >
>> > > > > > > on a related note, it could be helpful to have Jenkins
check our
>> > > > > release
>> > > > > > > tags
>> > > > > >
>> > > > > > That should be simple enough to add, since in pipelines
we can say
>> > stuff
>> > > > > > like "when { tag "release-*" }" where the wildcard is an
Ant-style
>> > > > > > wildcard. At the same time: since tags for us are always
on
>> > master, I
>> > > > > > mostly expect this to add no additional value, since we
already
>> > run tests
>> > > > > > on master pushes. Since tests on CI passed on the commit
you
>> > tagged for
>> > > > > the
>> > > > > > release, I expect the error to be somehow introduced by
the
>> > packaging
>> > > > > > process. In that case we'd want to run tests against the
RC source
>> > > > > > artifacts - this also shouldn't be too hard to add, but
would
>> > probably be
>> > > > > > best to set up as a job that's manually triggered against
a
>> > specific zip
>> > > > > > once it's uploaded (consider: we can automatically trigger
on
>> > tags, but
>> > > > > > more likely than not, the release at that point hasn't been
>> > uploaded to
>> > > > > the
>> > > > > > ASF dev repo yet). At that point this is "just" a convenience
/
>> > extra
>> > > > > > safety net, since voting PMC members will do this check
locally
>> > anyway.
>> > > > > >
>> > > > > > On Sun, Feb 10, 2019 at 10:38 AM Adrian Cole <
>> > adrian.f.cole@gmail.com>
>> > > > > > wrote:
>> > > > > >
>> > > > > > > thanks zoltan. we dont deploy the itests so probably
dont
>> > generally
>> > > > > need to
>> > > > > > > validate them for ASF reasons. however if there is
something in
>> > the
>> > > > > zip it
>> > > > > > > should work. I will look into this to see if it is
failing
>> > because it
>> > > > > > > should fail or not. also I will lightly look into how
much logic
>> > we
>> > > > > need in
>> > > > > > > general to strip itests out completely.
>> > > > > > >
>> > > > > > > on a related note, it could be helpful to have Jenkins
check our
>> > > > > release
>> > > > > > > tags (mvnw install not deploy) so that we know the
actual code
>> > still
>> > > > > works
>> > > > > > > (vs a distribution of it)
>> > > > > > >
>> > > > > > > On Sun, Feb 10, 2019, 5:20 PM Zoltán Nagy <abesto@apache.org
>> > wrote:
>> > > > > > >
>> > > > > > > > Confirming we're now in a better state:
>> > > > > > > >
>> > > > > > > > * GPG signature and SHA512 of the artifact check
out. I
>> > expected to
>> > > > > also
>> > > > > > > > see a signature for the checksum, but
>> > > > > > > > https://www.apache.org/dev/release-distribution
doesn't
>> > mention
>> > > > > that,
>> > > > > > > so I
>> > > > > > > > think we're fine.
>> > > > > > > > * Confirming base dir is now non-confusing :)
>> > > > > > > > * Code in release matches code at commit 4c28076fd
>> > > > > > > > * `./mvnw compile` succeeds (note: the license
checker Maven
>> > plugin
>> > > > > > > > complains for about a screenful about failing
to find the
>> > latest git
>> > > > > > > > commit, but this doesn't fail the build)
>> > > > > > > >
>> > > > > > > > However, I'm still -1:
>> > > > > > > >
>> > > > > > > > * `./mvnw package` fails both on my macOS and
Windows Linux
>> > Subsystem
>> > > > > > > > environments: io.zipkin.brave.itests.BraveTest
fails with
>> > > > > > > > java.lang.ClassNotFoundException for zipkin2.reporter.Sender.
>> > Gist of
>> > > > > > > > relevant Maven output:
>> > > > > > > >
>> > https://gist.github.com/abesto/632f7e7e515de2adb9b3ba04d7606659
>> > > > > > > >
>> > > > > > > > On Sun, Feb 10, 2019 at 4:10 AM Adrian Cole <
>> > adrian.f.cole@gmail.com
>> > > > > >
>> > > > > > > > wrote:
>> > > > > > > >
>> > > > > > > > > sorry I forgot to mention that GPG asc files
weren't there
>> > because
>> > > > > I
>> > > > > > > > > used the old "release" profile not the "apache-release"
one.
>> > I've
>> > > > > > > > > removed the old profile to remove confusion
as it is no
>> > longer used
>> > > > > > > > > anyway
>> > > > > > > > >
>> > > > > > > > > On Sun, Feb 10, 2019 at 5:08 AM Adrian Cole
<
>> > > > > adrian.f.cole@gmail.com>
>> > > > > > > > > wrote:
>> > > > > > > > > >
>> > > > > > > > > > OK all should be resolved now. The new
git hash is
>> > > > > > > > > > 4c28076fd617f4896cae77e773de7090bcebe6b4
>> > > > > > > > > >
>> > > > > > > > > > All other locations etc should be the
same. Here is a
>> > summary of
>> > > > > > > > > glitches fixed:
>> > > > > > > > > >
>> > > > > > > > > > * zip wasn't named correctly I formerly
manually fixed it.
>> > Now
>> > > > > that's
>> > > > > > > > > automatic
>> > > > > > > > > > * zip basedir wasn't intuitive. it is
now
>> > brave-karaf-$version
>> > > > > > > > > > * we accidentally published itests,
now we don't
>> > > > > > > > > > * dummy release notes didn't explain
this was only a canary
>> > > > > release
>> > > > > > > > > >
>> > > > > > > > > > There was no code change only build
script stuff.
>> > > > > > > > > >
>> > > > > > > > > > -A
>> > > > > > > > > >
>> > > > > > > > > > On Sat, Feb 9, 2019 at 10:38 PM Jorge
Quilcate <
>> > > > > > > > quilcate.jorge@gmail.com>
>> > > > > > > > > wrote:
>> > > > > > > > > > >
>> > > > > > > > > > > +1
>> > > > > > > > > > >
>> > > > > > > > > > > On 2/9/19 2:30 PM, Adrian Cole
wrote:
>> > > > > > > > > > > > agreed the release notes link
is empty. I didn't go
>> > through
>> > > > > the
>> > > > > > > > > formality
>> > > > > > > > > > > > of making release notes for
0.1.2
>> > > > > > > > > > > >
>> > > > > > > > > > > > On Sat, Feb 9, 2019, 9:19
PM Brian Devins-Suresh <
>> > > > > > > > badevins@gmail.com
>> > > > > > > > > wrote:
>> > > > > > > > > > > >
>> > > > > > > > > > > >> +1
>> > > > > > > > > > > >>
>> > > > > > > > > > > >> Are release votes not
supposed to include some
>> > synopsis of
>> > > > > the
>> > > > > > > > > changes? I
>> > > > > > > > > > > >> know this is a first release
within the incubator but
>> > it
>> > > > > might
>> > > > > > > be
>> > > > > > > > > good to
>> > > > > > > > > > > >> just call that out?
>> > > > > > > > > > > >>
>> > > > > > > > > > > >> On Sat, Feb 9, 2019 at
6:41 AM José Carlos Chávez <
>> > > > > > > > > jcchavezs@gmail.com>
>> > > > > > > > > > > >> wrote:
>> > > > > > > > > > > >>
>> > > > > > > > > > > >>> +1
>> > > > > > > > > > > >>>
>> > > > > > > > > > > >>> Den lør. 9. feb.
2019, 11:36 skrev Adrian Cole <
>> > > > > > > > > adrian.f.cole@gmail.com:
>> > > > > > > > > > > >>>
>> > > > > > > > > > > >>>>> I can't find
the GPG signature for the artifact
>> > and the
>> > > > > > > > > checksum. I'm
>> > > > > > > > > > > >>>>> looking at
>> > > > > > > > > > > >>>>>
>> > > > > > > > > > > >>
>> > > > > > > > >
>> > > > > > > >
>> > > > > > >
>> > > > >
>> > https://dist.apache.org/repos/dist/dev/incubator/zipkin/brave-karaf/0.1.2/
>> > > > > > > > > > > >>>>> ,
>> > > > > > > > > > > >>>>> and am expecting
to see two .asc files, and am not
>> > seeing
>> > > > > > > any.
>> > > > > > > > > Am I
>> > > > > > > > > > > >>>> looking
>> > > > > > > > > > > >>>>> in the wrong
place?
>> > > > > > > > > > > >>>>>
>> > > > > > > > > > > >>>> no that is the
right place. I must have missed
>> > something.
>> > > > > the
>> > > > > > > > > stuff
>> > > > > > > > > > > >> asked
>> > > > > > > > > > > >>>> me for my GPG
password so something must have
>> > gotten lost.
>> > > > > > > will
>> > > > > > > > > report
>> > > > > > > > > > > >>>> back.
>> > > > > > > > > > > >>>>
>> > > > > > > > > > > >>>> The folder contained
in the source zip is called
>> > > > > > > > > > > >>>>> "brave-karaf-parent-0.1.2".
I'd expect it to be
>> > just
>> > > > > > > > > > > >>> "brave-karaf-0.1.2".
>> > > > > > > > > > > >>>>> (Quite possible
I'm just missing some Java
>> > ecosystem
>> > > > > > > knowledge
>> > > > > > > > > and
>> > > > > > > > > > > >> this
>> > > > > > > > > > > >>>> is
>> > > > > > > > > > > >>>>> fine).
>> > > > > > > > > > > >>>> I think this is
also valid. Let me look into
>> > customizing
>> > > > > the
>> > > > > > > > > artifact
>> > > > > > > > > > > >>>> basedir. It is
inheriting this from the aggregator
>> > (base
>> > > > > > > > pom.xml)
>> > > > > > > > > which
>> > > > > > > > > > > >>> we
>> > > > > > > > > > > >>>> intentionally
name different as often the actual
>> > lib is
>> > > > > called
>> > > > > > > > > > > >> something
>> > > > > > > > > > > >>>> like brave-karaf
>> > > > > > > > > > > >>>>
>> > > > > > > > > > >
>> > > > > > > > > > >
>> > > > > > >
>> > ---------------------------------------------------------------------
>> > > > > > > > > > > To unsubscribe, e-mail:
>> > dev-unsubscribe@zipkin.apache.org
>> > > > > > > > > > > For additional commands, e-mail:
>> > dev-help@zipkin.apache.org
>> > > > > > > > > > >
>> > > > > > > > >
>> > > > > > > > >
>> > > > > ---------------------------------------------------------------------
>> > > > > > > > > To unsubscribe, e-mail: dev-unsubscribe@zipkin.apache.org
>> > > > > > > > > For additional commands, e-mail: dev-help@zipkin.apache.org
>> > > > > > > > >
>> > > > > > > > >
>> > > > > > > >
>> > > > > > >
>> > > > >
>> > > > > ---------------------------------------------------------------------
>> > > > > To unsubscribe, e-mail: dev-unsubscribe@zipkin.apache.org
>> > > > > For additional commands, e-mail: dev-help@zipkin.apache.org
>> > > > >
>> > > > >
>> >
>> > ---------------------------------------------------------------------
>> > To unsubscribe, e-mail: dev-unsubscribe@zipkin.apache.org
>> > For additional commands, e-mail: dev-help@zipkin.apache.org
>> >
>> >

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@zipkin.apache.org
For additional commands, e-mail: dev-help@zipkin.apache.org


Mime
View raw message