provide minorversion label on AppEngine by ofrobots · Pull Request #237 · googleapis/cloud-debug-nodejs

ofrobots · 2017-02-07T23:42:57Z

AppEngine flex no longer provides the GAE_MINOR_VERSION environment variable, but the debug UI depends upon it. Provide a fake value, but only on flex.

cristiancavalli

LGTM, just a quick couple questions:

test/test-debuglet.js

+           var debuglet = new Debuglet(debug, defaultConfig);
+           assert.ok(debuglet.config_);
+           assert.ok(debuglet.config_.serviceContext);
+           assert.ok(!debuglet.config_.serviceContext.minorVersion);


test/test-debuglet.js

+
+      it('should provide minorversion upon registration if on flex', function(
+                                                                         done) {
+        process.env.GAE_SERVICE = 'fake-service';


test/test-debuglet.js

+               {projectId: 'fake-project', credentials: fakeCredentials});
+           var debuglet = new Debuglet(debug, defaultConfig);
+
+           var scope = nock(API).post(REGISTER_PATH, function(body) {


src/agent/debuglet.js

    }
+
+    if (_.isString(serviceContext.minorVersion)) {
+      //          v--- intentional lowercase


test/test-debuglet.js

+
+      it('should conjure a fake minor version when running on flex',
+         function() {
+           process.env.GAE_SERVICE = 'fake-gae-service';


matthewloring

LGTM assuming you have verified that this addresses the warning message that was displayed.

ofrobots · 2017-02-08T00:42:37Z

The fix seems to be working.

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 7, 2017

cristiancavalli approved these changes Feb 7, 2017

View reviewed changes

matthewloring reviewed Feb 7, 2017

View reviewed changes

src/agent/debuglet.js

}

if (_.isString(serviceContext.minorVersion)) {

// v--- intentional lowercase

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

ofrobots force-pushed the fake-minor-version branch from df1df14 to 5a2ce08 Compare February 8, 2017 00:07

DominicKramer reviewed Feb 8, 2017

View reviewed changes

test/test-debuglet.js

it('should conjure a fake minor version when running on flex',

function() {

process.env.GAE_SERVICE = 'fake-gae-service';

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

matthewloring approved these changes Feb 8, 2017

View reviewed changes

provide minorversion label on AppEngine

1e79103

ofrobots force-pushed the fake-minor-version branch from 5a2ce08 to 1e79103 Compare February 8, 2017 00:43

ofrobots merged commit f651b8e into googleapis:master Feb 8, 2017

ofrobots deleted the fake-minor-version branch February 8, 2017 00:49

Conversation

ofrobots commented Feb 7, 2017

Uh oh!

cristiancavalli left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

matthewloring left a comment

Choose a reason for hiding this comment

Uh oh!

ofrobots commented Feb 8, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants