Adds cache option to uplinks #132

silkentrance · 2017-03-03T23:41:31Z

Open to discussion. Please review.

WIP: needs tests

[GH-131] add cache option to uplinks

jachstet-sea · 2017-03-06T08:32:15Z

lib/storage.js

@@ -246,7 +246,11 @@ Storage.prototype.get_tarball = function(name, filename) {
      }, self.config)
    }

-    var savestream = self.local.add_tarball(name, filename)
+    var savestream
+    var docache = !!uplink.cache


Just curious: Any reason to have a double negation here?

Yes, you are correct, this is rather unneccessary. Out of habit, I think...

juanpicado · 2017-05-23T17:05:39Z

I'm considering to merge this one, but I'd need a rebase from master and unit testing. Could you take a look #142 @silkentrance ?

silkentrance · 2017-06-01T16:54:22Z

@juanpicado Will do as soon as I find the time. Is tomorrow ok with you?

juanpicado · 2017-06-01T16:58:07Z

@silkentrance sure, no problem, take your time 👍

silkentrance · 2017-06-01T19:49:10Z

I also fixed the existing integration tests, namely I adjusted the sinopia-test-1.2.3.tgz package to match the requirements, renaming everything to verdaccio.

I also added test/integration/config_nocache.yaml and test/integration/test_nocache.pl.

I think that these integration tests need to be re-implemented 😁

The configuration uplink caching behavior is tested in test/unit/config.js.

codecov · 2017-06-01T19:51:41Z

Codecov Report

Merging #132 into master will increase coverage by 0.05%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #132      +/-   ##
==========================================
+ Coverage   79.12%   79.18%   +0.05%     
==========================================
  Files          24       24              
  Lines        2247     2253       +6     
==========================================
+ Hits         1778     1784       +6     
  Misses        469      469

Impacted Files	Coverage Δ
lib/storage.js	`83.94% <100%> (+0.3%)`	⬆️
lib/config.js	`98.07% <100%> (+0.03%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c274168...7018fc9. Read the comment docs.

juanpicado · 2017-06-01T20:05:34Z

Awesome, I'll review it soon 👍

silkentrance · 2017-06-01T20:20:24Z

ATM I am working on implementing the functional tests, but this may take a while. I need to get my head around the test cases first.

silkentrance · 2017-06-02T17:15:55Z

@juanpicado I am having a hard time with the proper configuration of server3 (config-3.yaml) in test/functional. It will always fail with a 404 on get_package.

While the integration test works as expected, I cannot get it to work in the functional tests.

Maybe you can give me some hints on how to use the functional server test API?

juanpicado

I'll need your feedback about the perl file, I have no clue :) the "why".

juanpicado · 2017-06-02T18:56:02Z

lib/storage.js

          _autogenerated: true,
        }, self.config);
      }
-      let savestream = self.local.add_tarball(name, filename);
+      let savestream = null;
+      if (uplink.config.cache == true) {


Do we need == for true?

juanpicado · 2017-06-02T18:57:06Z

test/functional/gh131.js

+const PKG_NOCACHE = 'pkg_nocache';
+const PKG_NOCACHE2 = 'pkg_nocache2';
+
+function iscached(pkgname, tarballname) {


camelCase here please, isCache. The same in other functions. I'll enable camelCase eslint rule soon.

juanpicado · 2017-06-02T19:00:13Z

test/functional/gh131.js

+  });
+
+  before(function() {
+    let pkg = require('./lib/package')(PKG_NOCACHE2);


const if the variable does not change

juanpicado · 2017-06-02T19:01:14Z

test/functional/lib/startup.js

@@ -41,4 +42,5 @@ module.exports.start = function(dir, conf) {
 process.on('exit', function() {
  if (forks[0]) forks[0].kill();


Let's use curly braces, I'll enable that rule soon as well.

juanpicado · 2017-06-02T19:02:04Z

test/unit/config.js

@@ -0,0 +1,17 @@
+'use strict';
+
+let assert = require('assert');


const for imports

juanpicado · 2017-06-02T19:02:36Z

test/unit/partials/config.js

@@ -2,6 +2,11 @@

 let config = {


const here as well

juanpicado · 2017-06-02T19:04:12Z

@silkentrance I'm testing your branch locally, to figure out the 404.

silkentrance · 2017-06-02T19:56:11Z

@juanpicado will fix. thanks for looking into this. 👍

juanpicado · 2017-06-03T09:22:44Z

@silkentrance the 404 seems to be because in your packages configuration for server3 you define pkg-nocache while in your test you define

const PKG_NOCACHE = 'pkg_nocache';
const PKG_NOCACHE2 = 'pkg_nocache2';

Thus, the uplinks are false, that drives you to 404. Then if you fix that the asserts are false, I do not know if the PKG_NOCACHE name is intended or not.

silkentrance · 2017-06-03T17:26:40Z

@juanpicado Thanks so much for Findling

silkentrance · 2017-06-03T17:28:03Z

@juanpicado Thank you so much for finding this annoying typo. I will fix that right away incl. the issues you found in your review.

silkentrance · 2017-06-08T15:51:58Z

Dunno why this was closed. Must have hit the wrong button...

silkentrance · 2017-06-08T16:00:19Z

@juanpicado fixed issues and tests. rebased to master.

juanpicado · 2017-06-08T17:21:33Z

Cool !! @silkentrance I'll review it asap

juanpicado · 2017-06-08T18:47:10Z

Awesome work, unit testing widely complete 👍 thanks so much ! This goes to the next version.

lock · 2018-05-31T01:54:45Z

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

jachstet-sea reviewed Mar 6, 2017

View reviewed changes

juanpicado mentioned this pull request May 14, 2017

clear cache endpoint? #142

Closed

juanpicado mentioned this pull request May 23, 2017

Cleaning up storage folder #200

Closed

silkentrance force-pushed the gh-131 branch 2 times, most recently from 2ec20fa to 62bd467 Compare June 1, 2017 19:48

silkentrance force-pushed the gh-131 branch from 62bd467 to f0e9184 Compare June 1, 2017 19:58

juanpicado added issue_needs_triage topic: proxy feat: storage labels Jun 1, 2017

juanpicado self-assigned this Jun 1, 2017

juanpicado added the dev: code-review label Jun 1, 2017

silkentrance force-pushed the gh-131 branch from f0e9184 to aa81c3f Compare June 2, 2017 17:13

juanpicado requested changes Jun 2, 2017

View reviewed changes

silkentrance closed this Jun 3, 2017

silkentrance reopened this Jun 8, 2017

silkentrance force-pushed the gh-131 branch from aa81c3f to 2fbf32e Compare June 8, 2017 15:58

[verdaccioGH-131] add cache option to uplinks

7018fc9

silkentrance force-pushed the gh-131 branch from 2fbf32e to 7018fc9 Compare June 8, 2017 16:00

juanpicado approved these changes Jun 8, 2017

View reviewed changes

juanpicado merged commit 0cabd62 into verdaccio:master Jun 8, 2017

juanpicado added this to the 2.1.8 milestone Jun 8, 2017

silkentrance deleted the gh-131 branch June 8, 2017 19:19

juanpicado mentioned this pull request Jun 10, 2017

Add cache option to uplinks #131

Closed

lock bot locked as resolved and limited conversation to collaborators May 31, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds cache option to uplinks #132

Adds cache option to uplinks #132

silkentrance commented Mar 3, 2017

jachstet-sea Mar 6, 2017

silkentrance Mar 6, 2017

juanpicado commented May 23, 2017 •

edited

Loading

silkentrance commented Jun 1, 2017 •

edited

Loading

juanpicado commented Jun 1, 2017

silkentrance commented Jun 1, 2017

codecov bot commented Jun 1, 2017 •

edited

Loading

juanpicado commented Jun 1, 2017 •

edited

Loading

silkentrance commented Jun 1, 2017

silkentrance commented Jun 2, 2017

juanpicado left a comment

juanpicado Jun 2, 2017

juanpicado Jun 2, 2017

juanpicado Jun 2, 2017

juanpicado Jun 2, 2017

juanpicado Jun 2, 2017

juanpicado Jun 2, 2017

juanpicado commented Jun 2, 2017

silkentrance commented Jun 2, 2017

juanpicado commented Jun 3, 2017

silkentrance commented Jun 3, 2017

silkentrance commented Jun 3, 2017 •

edited

Loading

silkentrance commented Jun 8, 2017

silkentrance commented Jun 8, 2017

juanpicado commented Jun 8, 2017

juanpicado commented Jun 8, 2017

lock bot commented May 31, 2018

		@@ -41,4 +42,5 @@ module.exports.start = function(dir, conf) {
		process.on('exit', function() {
		if (forks[0]) forks[0].kill();

		@@ -0,0 +1,17 @@
		'use strict';

		let assert = require('assert');

Adds cache option to uplinks #132

Adds cache option to uplinks #132

Conversation

silkentrance commented Mar 3, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

juanpicado commented May 23, 2017 • edited Loading

silkentrance commented Jun 1, 2017 • edited Loading

juanpicado commented Jun 1, 2017

silkentrance commented Jun 1, 2017

codecov bot commented Jun 1, 2017 • edited Loading

Codecov Report

juanpicado commented Jun 1, 2017 • edited Loading

silkentrance commented Jun 1, 2017

silkentrance commented Jun 2, 2017

juanpicado left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

juanpicado commented Jun 2, 2017

silkentrance commented Jun 2, 2017

juanpicado commented Jun 3, 2017

silkentrance commented Jun 3, 2017

silkentrance commented Jun 3, 2017 • edited Loading

silkentrance commented Jun 8, 2017

silkentrance commented Jun 8, 2017

juanpicado commented Jun 8, 2017

juanpicado commented Jun 8, 2017

lock bot commented May 31, 2018

juanpicado commented May 23, 2017 •

edited

Loading

silkentrance commented Jun 1, 2017 •

edited

Loading

codecov bot commented Jun 1, 2017 •

edited

Loading

juanpicado commented Jun 1, 2017 •

edited

Loading

silkentrance commented Jun 3, 2017 •

edited

Loading