Re: [PROPOSAL] Add getCache and getLocator to Launchers
+1 for exposing getCache() on CacheLauncher instances. Death to all
singletons!
I'm less certain about the wisdom of exposing a getCache() on
LocatorLauncher instances. Seems like it would be better to let clients
call getLocator() on LocatorLauncher instances, then they can do the
traversal to the cache via getCache() themselves. Would it make sense to
expose getLocator() on LocatorLauncher instances (instead of exposing
getCache() on those)?
If we did implement getCache() for LocatorLauncher instances, it might look
something like this:
// on LocatorLauncher
public Optional getCache() {
return locator == null ? Optional.empty() :
Optional.of(locator.getCache());
}
That traversal seems more appropriate for the caller to implement. That way
the caller knows why the cache is unavailable (when its unavailable) e.g.
because there is no locator vs. because there is a locator but that locator
has no cache.
On Wed, Oct 31, 2018 at 3:05 PM Dan Smith wrote:
> Yay for APIs that don't require singletons!
>
> -Dan
>
> On Wed, Oct 31, 2018 at 2:54 PM Jinmei Liao wrote:
>
> > +1. sounds like a good addition and since we already have package level
> > getters for them anyway.
> >
> > On Wed, Oct 31, 2018 at 2:48 PM Kirk Lund wrote:
> >
> > > LocatorLauncher provides an API which can be used in-process to create
> a
> > > Locator. There is no public API on that class to get a reference to the
> > > Locator or its Cache.
> > >
> > > Similarly, ServerLauncher provides an API which can be used in-process
> to
> > > create a Server, but there is no public API in that class to get a
> > > reference to its Cache.
> > >
> > > The User of either Launcher would then have to resort to invoking
> > > singletons to get a reference to the Cache.
> > >
> > > There are existing package-private getter APIs on both Launchers but
> > > they're only used by tests in that same package.
> > >
> > > I propose adding public APIs for getCache to both LocatorLauncher and
> > > ServerLauncher as well as adding getLocator to LocatorLauncher. The
> > > signatures would look like:
> > >
> > > /**
> > > * Gets a reference to the Cache that was created by this
> ServerLauncher.
> > > *
> > > * @return a reference to the Cache
> > > */
> > > public org.apache.geode.cache.Cache getCache();
> > >
> > > /**
> > > * Gets a reference to the Locator that was created by this
> > > LocatorLauncher.
> > > *
> > > * @return a reference to the Locator
> > > */
> > > public org.apache.geode.distributed.Locator getLocator();
> > >
> > > Any thoughts? Yay or nay?
> > >
> > > Thanks,
> > > Kirk
> > >
> >
> >
> > --
> > Cheers
> >
> > Jinmei
> >
>
[Spring CI] Spring Data GemFire > Nightly-ApacheGeode > #1087 was SUCCESSFUL (with 2457 tests)
--- Spring Data GemFire > Nightly-ApacheGeode > #1087 was successful. --- Scheduled 2459 tests in total. https://build.spring.io/browse/SGF-NAG-1087/ -- This message is automatically generated by Atlassian Bamboo
Re: [PROPOSAL] Add getCache and getLocator to Launchers
Yay for APIs that don't require singletons! -Dan On Wed, Oct 31, 2018 at 2:54 PM Jinmei Liao wrote: > +1. sounds like a good addition and since we already have package level > getters for them anyway. > > On Wed, Oct 31, 2018 at 2:48 PM Kirk Lund wrote: > > > LocatorLauncher provides an API which can be used in-process to create a > > Locator. There is no public API on that class to get a reference to the > > Locator or its Cache. > > > > Similarly, ServerLauncher provides an API which can be used in-process to > > create a Server, but there is no public API in that class to get a > > reference to its Cache. > > > > The User of either Launcher would then have to resort to invoking > > singletons to get a reference to the Cache. > > > > There are existing package-private getter APIs on both Launchers but > > they're only used by tests in that same package. > > > > I propose adding public APIs for getCache to both LocatorLauncher and > > ServerLauncher as well as adding getLocator to LocatorLauncher. The > > signatures would look like: > > > > /** > > * Gets a reference to the Cache that was created by this ServerLauncher. > > * > > * @return a reference to the Cache > > */ > > public org.apache.geode.cache.Cache getCache(); > > > > /** > > * Gets a reference to the Locator that was created by this > > LocatorLauncher. > > * > > * @return a reference to the Locator > > */ > > public org.apache.geode.distributed.Locator getLocator(); > > > > Any thoughts? Yay or nay? > > > > Thanks, > > Kirk > > > > > -- > Cheers > > Jinmei >
Re: [PROPOSAL] Add getCache and getLocator to Launchers
+1. sounds like a good addition and since we already have package level getters for them anyway. On Wed, Oct 31, 2018 at 2:48 PM Kirk Lund wrote: > LocatorLauncher provides an API which can be used in-process to create a > Locator. There is no public API on that class to get a reference to the > Locator or its Cache. > > Similarly, ServerLauncher provides an API which can be used in-process to > create a Server, but there is no public API in that class to get a > reference to its Cache. > > The User of either Launcher would then have to resort to invoking > singletons to get a reference to the Cache. > > There are existing package-private getter APIs on both Launchers but > they're only used by tests in that same package. > > I propose adding public APIs for getCache to both LocatorLauncher and > ServerLauncher as well as adding getLocator to LocatorLauncher. The > signatures would look like: > > /** > * Gets a reference to the Cache that was created by this ServerLauncher. > * > * @return a reference to the Cache > */ > public org.apache.geode.cache.Cache getCache(); > > /** > * Gets a reference to the Locator that was created by this > LocatorLauncher. > * > * @return a reference to the Locator > */ > public org.apache.geode.distributed.Locator getLocator(); > > Any thoughts? Yay or nay? > > Thanks, > Kirk > -- Cheers Jinmei
[PROPOSAL] Add getCache and getLocator to Launchers
LocatorLauncher provides an API which can be used in-process to create a Locator. There is no public API on that class to get a reference to the Locator or its Cache. Similarly, ServerLauncher provides an API which can be used in-process to create a Server, but there is no public API in that class to get a reference to its Cache. The User of either Launcher would then have to resort to invoking singletons to get a reference to the Cache. There are existing package-private getter APIs on both Launchers but they're only used by tests in that same package. I propose adding public APIs for getCache to both LocatorLauncher and ServerLauncher as well as adding getLocator to LocatorLauncher. The signatures would look like: /** * Gets a reference to the Cache that was created by this ServerLauncher. * * @return a reference to the Cache */ public org.apache.geode.cache.Cache getCache(); /** * Gets a reference to the Locator that was created by this LocatorLauncher. * * @return a reference to the Locator */ public org.apache.geode.distributed.Locator getLocator(); Any thoughts? Yay or nay? Thanks, Kirk
Re: Pull request precheckin not firing automatically
Just in case, others hit this: The one I was asking about failed with "missing inputs: geode, instance-data" which Dan said means that Concourse barfed. When this happens, pushing an empty commit or anything else was recommended to re-trigger. https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-pr/jobs/AcceptanceTest/builds/293 Thanks, Kirk On Wed, Oct 31, 2018 at 11:19 AM, Patrick Rhomberg wrote: > Just to disseminate the knowledge... > > Although we like it when everyone just works nicely, you can check the > consumption of your PR in the Concourse by looking at the *geode* resource > in the *apache-develop-pr* pipeline [1]. This resource passes the PR > number and associated SHA to test against, so you can search for your PR in > this list. > > The first thing that the jobs are meant to do to add Check Status hooks to > GitHub that let us click into the specific job, but you can see in the > *geode* resource if a job has suffered infrastructure failures. For > instance, clicking on *0c5f7* or *ccd90* of your *pr 2730*, I see that the > jobs failed to launch due to merge conflicts. (Aside [2].) > > The PR precheckin is always run against the "if this were merged" version > of Geode. If a precheckin doesn't fire, it is often because there are > merge conflicts that must be resolved first. Merge origin/develop into > your branch and push to your fork, and you should be good to go. > And if you use a merge rather than a rebase, you don't have the history > (such as the SHAs I referenced above) disappear on you. > > To head off another potential source of confusion when looking at the > Concourse resource, you might notice that some SHAs in your *geode* > resource > history will be skipped if they are immediately identified as older than > the PR's current HEAD. For instance, your *pr 2730* with SHA *98491* > didn't > get a precheckin run, since it was immediately superseded by the newer > *5ffc0* commit. > > But, the bottom line is: make sure you're merged with *origin/develop* when > you open / push your PR and precheckin should (tm) consistently fire, > barring other infrastructure instability. > > Hope that helps! > > Imagination is Change. > ~Patrick > > [1] > https://concourse.apachegeode-ci.info/teams/main/pipelines/ > apache-develop-pr/resources/geode > [2] Currently, we don't get the GitHub hook when there are merge conflicts > because the Concourse resource acquisition itself fails and we never reach > the task in Concourse to update the GitHub hooks. We should probably > investigate if there is a way to add the hooks in the case of a merge > conflict, to avoid the potential for developer confusion. > > On Tue, Oct 30, 2018 at 4:16 PM, Kirk Lund wrote: > > > Nevermind. I pushed again and it seems to have triggered this time. > > > > On Tue, Oct 30, 2018 at 2:51 PM, Kirk Lund wrote: > > > > > I have a PR that I updated a while ago, but it's not automatically > firing > > > a precheckin. > > > > > > What's the expected behavior? Is it supposed to automatically trigger a > > > precheckin if I push more changes? > > > > > > Here's my PR: https://github.com/apache/geode/pull/2730 > > > > > > PS: the PR isn't ready to actually merge, it's just the only way I know > > to > > > run a precheckin now > > > > > > Thanks, > > > Kirk > > > > > > > > >
Re: Pull request precheckin not firing automatically
Just to disseminate the knowledge... Although we like it when everyone just works nicely, you can check the consumption of your PR in the Concourse by looking at the *geode* resource in the *apache-develop-pr* pipeline [1]. This resource passes the PR number and associated SHA to test against, so you can search for your PR in this list. The first thing that the jobs are meant to do to add Check Status hooks to GitHub that let us click into the specific job, but you can see in the *geode* resource if a job has suffered infrastructure failures. For instance, clicking on *0c5f7* or *ccd90* of your *pr 2730*, I see that the jobs failed to launch due to merge conflicts. (Aside [2].) The PR precheckin is always run against the "if this were merged" version of Geode. If a precheckin doesn't fire, it is often because there are merge conflicts that must be resolved first. Merge origin/develop into your branch and push to your fork, and you should be good to go. And if you use a merge rather than a rebase, you don't have the history (such as the SHAs I referenced above) disappear on you. To head off another potential source of confusion when looking at the Concourse resource, you might notice that some SHAs in your *geode* resource history will be skipped if they are immediately identified as older than the PR's current HEAD. For instance, your *pr 2730* with SHA *98491* didn't get a precheckin run, since it was immediately superseded by the newer *5ffc0* commit. But, the bottom line is: make sure you're merged with *origin/develop* when you open / push your PR and precheckin should (tm) consistently fire, barring other infrastructure instability. Hope that helps! Imagination is Change. ~Patrick [1] https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-pr/resources/geode [2] Currently, we don't get the GitHub hook when there are merge conflicts because the Concourse resource acquisition itself fails and we never reach the task in Concourse to update the GitHub hooks. We should probably investigate if there is a way to add the hooks in the case of a merge conflict, to avoid the potential for developer confusion. On Tue, Oct 30, 2018 at 4:16 PM, Kirk Lund wrote: > Nevermind. I pushed again and it seems to have triggered this time. > > On Tue, Oct 30, 2018 at 2:51 PM, Kirk Lund wrote: > > > I have a PR that I updated a while ago, but it's not automatically firing > > a precheckin. > > > > What's the expected behavior? Is it supposed to automatically trigger a > > precheckin if I push more changes? > > > > Here's my PR: https://github.com/apache/geode/pull/2730 > > > > PS: the PR isn't ready to actually merge, it's just the only way I know > to > > run a precheckin now > > > > Thanks, > > Kirk > > > > >
Geode 1.8 Release Manager
We are coming up on the date where we said we would start the 1.8 release (Nov 1st). Any volunteers to be release manager for this release? -Dan
