node-red / flow-library

Node-RED Flow Library
Apache License 2.0
70 stars 44 forks source link

Scorecard showing old results #89

Open sammachin opened 2 years ago

sammachin commented 2 years ago

node-red-contrib-midi was updated to 1.1.2 on 13/05/2022 with fixes for the license and versions,

However the scorecard ran but still showed the incorrect results Screen Shot 2022-05-16 at 3 09 44 PM

Validation locally with node-red-dev shows that it should have passes for license and versions.

pbrunier commented 12 months ago

I have the same issue. Yesterday I published a new version with correct license and today the scorecard still reflects the old version. https://flows.nodered.org/node/node-red-contrib-dyson-purelink

Running node-red-dev locally shows it is ok: node-red-contrib-dyson-purelink@0.6.1 ✅ Package is MIT licensed

marcus-j-davies commented 9 months ago

Hello,

I apologize if this seems pushy, its not intended to be, only many many Node devs including my myself, have made improvements to our packages, as we get scolded by the scorecard.

Only to find out the scorecard is not updating and our packages still being scored down (aggressively) despite meeting certain standards.

Many many people on the forums also raise this same issue, and I have also added comments, onto various discussions that are on the forums, without much feedback on the issue.

This issue is now more than a year old, and I cant see much in the way of any focus into getting it sorted.

I was looking through the codebase of the flow library, to try and get it fixed myself (as I could imagine your all busy), but seems the codebase is more intricate then I can understand.

The scorecard does, I feel, make quite a difference into the level of quality seen by users.

Once again, sorry if this seems flippant - it really isn't meant to , but maybe an update on the issue will be welcome, given the impact a scorecard can have.

knolleary commented 9 months ago

You are quite right @marcus-j-davies - we need to get to the bottom of this. I'll revisit it this week.

marcus-j-davies commented 9 months ago

Thanks @knolleary,

I'm all for prioritisation, but this one has been going on for sometime, and seeing it crop up more often lately, plus I released a 2nd Node, only to receive 3rd degree burns by the scorecard, and subsequently corrected of course, but not earning the "well done" badge 😅

knolleary commented 9 months ago

I've just pushed some extra debug into production to see if I can make any sense of what is going on.

What is truly confusing is that the way this works is:

  1. flow library downloads the tar file of the module, unpacks it, extracts all its information and updates database.
  2. it then points the scorecard tool at the same directory that contains the just-downloaded version - but for some reason the scorecard 'sees' a different version.

Hoping the extra debug I've added sheds some light.

marcus-j-davies commented 9 months ago

Hi @knolleary

This is of interest to me...

The validate method in the dev-clicommand employees getFromNPM but it checks to see if the package has already been saved to temp (say in a previous run first)

https://github.com/node-red/node-red-dev-cli/blob/2595fbb2bfc4b96f3de0963ea660831c8f79122f/src/libs/npmget.js#L9

The call to getFromNPM https://github.com/node-red/node-red-dev-cli/blob/2595fbb2bfc4b96f3de0963ea660831c8f79122f/src/commands/validate.js#L40

EDIT On second review - I think it just creates the directory

knolleary commented 9 months ago

12 hours later, we've had a number of nodes refresh in the library and none have exhibited the mismatch behaviour... so we continue to wait.

sammachin commented 9 months ago

@knolleary I just published some updates to my package @sammachin/node-red-matter-bridge the first update to 0.0.3 refreshed the scorecard correctly but then a subsequent update to fix the package.json for the node-red version in 0.0.4 and 0.0.5 doesn't seem to have been picked up.

knolleary commented 9 months ago

Thanks @sammachin

From the debug I added last night I can see the following:

Scorecard package.json: @sammachin/node-red-matter-bridge 0.0.5
---Validating Package---
@sammachin/node-red-matter-bridge@0.0.3

The first line is a new line I added - it loads the package.json file on disk and confirms the version number inside it - 0.0.5. The last line is from the scorecard tool - reporting it has evaluated 0.0.3.

I have logged into the VM hosting the flow library and checked the files on disk - sure enough only 0.0.5 is present. And when I manually run the scorecard with the same args as the flow library used, I get the 0.0.5 scorecard.

I'm going to have to patch the scorecard tool with some extra debug to figure this out.

sammachin commented 9 months ago

thats very odd, if I remember correctly it downloads the package to a tmp folder with a unique name too? (some random chars added to the end) I wonder if the scorecard tool isn't closing things properly after the first run?

marcus-j-davies commented 9 months ago

Another avenue that maybe of interest.

The checks make a lot of require calls on the package

On a historical project of mine I needed a way to invalidate the nodejs cache via. delete require.cache[require.resolve(...)]

leaning towards the comment by @sammachin - is the process a singleton?, as surely the cache should be cleared after the process exits?

to a tmp folder with a unique name too?

I don't think it does, but then I'm not that familiar with the codebase https://github.com/node-red/node-red-dev-cli/blob/2595fbb2bfc4b96f3de0963ea660831c8f79122f/src/libs/npmget.js#L7

knolleary commented 9 months ago

@marcus-j-davies 🎉 that'll explain it.

marcus-j-davies commented 9 months ago

Now now Nick! - could be a red herring 😅

knolleary commented 9 months ago

Okay - 0.1.6 of node-red-dev published with the fix.

Flow Library updated to use that... deploy in progress. Now we wait and see. If this theory is right (and I'm 99.9% sure it is) then we have to wait for a module to do its second refresh after this restart to confirm all is well.

sammachin commented 9 months ago

I don't think it does, but then I'm not that familiar with the codebase

@marcus-j-davies The way the flow library works is different to locally is that it has already downloaded the package from npm so the scorecard tool is pointed at the local folder rather than downloading from NPM itself. The scorecard tool is run as part of the flow library process then so I can see that the require cache would be an issue here that we don't pickup when running it from a shell as a new process.

marcus-j-davies commented 9 months ago

@knolleary,

I am afraid this hasn't worked,

I have just pushed 2 updates the first I purposely removed license, and fixed it in the 2nd

Screenshot 2023-09-23 at 09 59 41

Screenshot 2023-09-23 at 10 04 04

What I did notice - delete require.cache[require.resolve(...)] appears after the require, not sure if that is key

at least what works for me, is clearing before requiring it https://github.com/marcus-j-davies/HAP-Router/blob/ba7f8d472fe28d74a011a6f6d9e390e3d0338392/core/server.js#L552

EDIT Interesting... Just uploaded a 3rd version - and its updated.

Screenshot 2023-09-23 at 11 56 40

sammachin commented 9 months ago

Just released a couple of updates to my matter-bridge node this morning, first one 0.0.6 didn't have any changes to the scorecard compared to 0.0.5 (correct), then published 0.0.8 about 3 hours later where I'd added examples and the scorecard has been updated. So seems to be working for me at least Screen Shot 2023-10-09 at 1 10 01 PM Screen Shot 2023-10-09 at 1 09 56 PM