Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Scorecard showing old results #89

Open
sammachin opened this issue May 16, 2022 · 17 comments
Open

Scorecard showing old results #89

sammachin opened this issue May 16, 2022 · 17 comments
Assignees

Comments

@sammachin
Copy link
Collaborator

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
Copy link

pbrunier commented Jul 9, 2023

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:
[email protected]
✅ Package is MIT licensed

@marcus-j-davies
Copy link

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
Copy link
Member

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
Copy link

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
Copy link
Member

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.

@knolleary knolleary assigned knolleary and unassigned sammachin Sep 19, 2023
@marcus-j-davies
Copy link

marcus-j-davies commented Sep 19, 2023

Hi @knolleary

This is of interest to me...

The validate method in the dev-cli command 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
Copy link
Member

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
Copy link
Collaborator Author

@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
Copy link
Member

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/[email protected]

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
Copy link
Collaborator Author

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
Copy link

marcus-j-davies commented Sep 20, 2023

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
Copy link
Member

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

@marcus-j-davies
Copy link

Now now Nick! - could be a red herring 😅

@knolleary
Copy link
Member

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
Copy link
Collaborator Author

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
Copy link

marcus-j-davies commented Sep 23, 2023

@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
Copy link
Collaborator Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants