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.

Sign up for GitHub

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

Jump to bottom

Create purldb scan worker #1078

Merged
merged 30 commits into from
Mar 27, 2024
Merged

Create purldb scan worker #1078

merged 30 commits into from
Mar 27, 2024

Conversation

pombredanne
Copy link
Member

@pombredanne pombredanne commented Feb 15, 2024 •
edited by mjherzog

This PR is the ScanCode.io client side of the PurlDB scanning queue as designed in nexB/purldb#236

@pombredanne pombredanne marked this pull request as draft February 15, 2024 10:02
@pombredanne pombredanne marked this pull request as ready for review February 15, 2024 10:03
@pombredanne pombredanne mentioned this pull request Feb 15, 2024
Merged
@tdruez
Copy link
Member

tdruez commented Feb 19, 2024

  1. Tests are failing
  2. TODOs to be addressed
  3. Lack of unit tests for the new code

@tdruez tdruez marked this pull request as draft February 19, 2024 10:29
@tdruez
Copy link
Member

tdruez commented Feb 26, 2024

@JonoYang What's the latest status on this one?

@JonoYang
Copy link
Contributor

@tdruez

I was working on updates to the purldb and API. I've updated my management command in this PR to make sure that basic functionality works (get download url, scan package, send results back), and that we can report scan project failures to purldb. I still need to create tests for management command functions.

@pombredanne pombredanne mentioned this pull request Feb 28, 2024
Closed
@pombredanne
Copy link
Member Author

This needs rebasing/merging once #1077 is merged ...because of the renamings.

@pombredanne pombredanne mentioned this pull request Mar 12, 2024
Closed
9 tasks
@JonoYang JonoYang requested a review from tdruez March 20, 2024 02:03
tdruez
tdruez requested changes Mar 20, 2024
View reviewed changes
Copy link
Member

@tdruez tdruez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JonoYang See my comments.
Also, should #1077 be merged first or after this one?

scanpipe/management/commands/__init__.py Outdated Show resolved Hide resolved
scanpipe/management/commands/purldb-scan-queue-worker.py Outdated Show resolved Hide resolved
scanpipe/management/commands/purldb-scan-queue-worker.py
"0 means no limit. Used only for testing.",
)

def handle(self, *args, **options):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is too complex and needs to be refactored in smaller pieces

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've refactored the code in the handle() method. Is what I have still too complex?

scanpipe/pipes/purldb.py Outdated Show resolved Hide resolved
scanpipe/pipes/purldb.py Outdated Show resolved Hide resolved
scanpipe/pipes/purldb.py Outdated Show resolved Hide resolved
scanpipe/pipes/purldb.py Outdated Show resolved Hide resolved
scanpipe/tests/test_commands.py
Comment on lines +787 to +790
class CreateProjectCommand(
commands.CreateProjectCommandMixin, commands.AddInputCommandMixin, BaseCommand
):
pass
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This setup seems too complex.

Copy link
Contributor

@JonoYang JonoYang Mar 21, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tdruez

Do you have any suggestions on how I should test the command methods? Should I have just called create-project using call_command instead of putting the project creation logic into a method?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about having the functions related to package creation outside the Command class? Those could be at the module level, what's your take?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tdruez I am on board with that idea. What should be done about the messages that are printed using styling added by the django Command class? For instance, the calls to self.stdout.write() and self.stderr.write()? I think that's the main reason why I kept this code as Command mixins

@JonoYang JonoYang force-pushed the scio-purldb-scan-worker branch 2 times, most recently from 1360a05 to aa4427d Compare March 23, 2024 00:36
@JonoYang JonoYang requested a review from tdruez March 23, 2024 00:48
@JonoYang
Copy link
Contributor

Also, should #1077 be merged first or after this one?

I think its alright to merge them in any order

@JonoYang
Create management command for purldb package scanning
a25c41c 
Signed-off-by: Jono Yang <jyang@nexb.com>
@JonoYang
Fix bugs in package-scan-worker
91cb76c 
Signed-off-by: Jono Yang <jyang@nexb.com>
@JonoYang
Send updates to purldb regarding scan status
c31e7e7 
Signed-off-by: Jono Yang <jyang@nexb.com>
@JonoYang
Return scan project url when updating scan status on purldb
6bad6bc 
Signed-off-by: Jono Yang <jyang@nexb.com>
@JonoYang
Get scananble_uri_uuid instead of package_uuid
bad9d62 
     * Add sleep to main work loop

Signed-off-by: Jono Yang <jyang@nexb.com>
@JonoYang
Move project creation logic into its own function
5752639 
    * Handle exceptions in package-scan-worker and send to purldb as errors

Signed-off-by: Jono Yang <jyang@nexb.com>
@JonoYang
Create project name from download url and uuid
6d65cb5 
Signed-off-by: Jono Yang <jyang@nexb.com>
@JonoYang
Refactor poll_until_success
65ab289 
Signed-off-by: Jono Yang <jyang@nexb.com>
@JonoYang
Add tests for new purldb functions
4b70406 
Signed-off-by: Jono Yang <jyang@nexb.com>
@JonoYang
Collect possible exceptions when getting next job
7a7fd04 
Signed-off-by: Jono Yang <jyang@nexb.com>
@JonoYang
Get pipelines to run from purldb
05a684b 
Signed-off-by: Jono Yang <jyang@nexb.com>
@JonoYang
Do not use sys.exit() in execute.py
2888386 
Signed-off-by: Jono Yang <jyang@nexb.com>
@JonoYang
Add async arg to package-scan-worker.py
4bebc21 
Signed-off-by: Jono Yang <jyang@nexb.com>
@JonoYang
Refactor commonly used command functions
1fffca4 
Signed-off-by: Jono Yang <jyang@nexb.com>
@JonoYang
Create tests for Command mixins
c494f02 
Signed-off-by: Jono Yang <jyang@nexb.com>
@JonoYang
Move get_next_job() to pipes/purldb.py
d500b5e 
Signed-off-by: Jono Yang <jyang@nexb.com>
@JonoYang
Create test for package-scan-worker
085d034 
Signed-off-by: Jono Yang <jyang@nexb.com>
@JonoYang
Create failure tests for package-scan-worker
38bf973 
    * Remove `scanscan_project_url` from scanpipe.pipes.purldb.update_status()

Signed-off-by: Jono Yang <jyang@nexb.com>
@JonoYang
Save scannable_uri_uuid to project extra data
bc1fbed 
Signed-off-by: Jono Yang <jyang@nexb.com>
@JonoYang
Bump matchcode-toolkit version
c6d50e6 
Signed-off-by: Jono Yang <jyang@nexb.com>
@JonoYang
Send summary along with scan results to purldb
ccfc673 
Signed-off-by: Jono Yang <jyang@nexb.com>
@JonoYang
Rename arguments for send_results_to_purldb
a7f48ee 
    * Bump matchcode-toolkit version to 4.0.0

Signed-off-by: Jono Yang <jyang@nexb.com>
@JonoYang
Send project extra data to purldb with scan results
4401aea 
Signed-off-by: Jono Yang <jyang@nexb.com>
@JonoYang
Print success message when results are sent
faa0957 
Signed-off-by: Jono Yang <jyang@nexb.com>
@JonoYang
Update package-scan-worker tests
709d171 
    * Sen both traceback and exception message to purldb

Signed-off-by: Jono Yang <jyang@nexb.com>
@JonoYang
Update CHANGELOG.rst
5c85dc6 
    * Update management command test names

Signed-off-by: Jono Yang <jyang@nexb.com>
@JonoYang
Address review comments
1d472d5 
    * Move lists of statuses in poll_until_success to their own variables
    * Remove unnecessary else statement
    * Use tuples as default values for `create_project`

Signed-off-by: Jono Yang <jyang@nexb.com>
@JonoYang
Refactor logic in purldb-scan-queue-worker
3c81a2d 
    * Rename get_next_job to get_next_download_url
    * Update tests

Signed-off-by: Jono Yang <jyang@nexb.com>
@JonoYang
Fix imports and tests after rebase
27ad732 
Signed-off-by: Jono Yang <jyang@nexb.com>
@JonoYang
Move Command methods into functions
2fb7e12 
Signed-off-by: Jono Yang <jyang@nexb.com>
@tdruez tdruez marked this pull request as ready for review March 27, 2024 11:17
@tdruez tdruez merged commit 4f67f97 into main Mar 27, 2024
7 checks passed
@tdruez tdruez deleted the scio-purldb-scan-worker branch March 27, 2024 11:17
@JonoYang JonoYang mentioned this pull request Mar 28, 2024
Closed
7 tasks
@tdruez tdruez mentioned this pull request Mar 29, 2024
Closed
@pombredanne pombredanne mentioned this pull request Apr 3, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JonoYang JonoYang Awaiting requested review from JonoYang

@tdruez tdruez Awaiting requested review from tdruez

Assignees

@JonoYang JonoYang

Labels
None yet
Projects
02-MatchCode - More matching
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@pombredanne @tdruez @JonoYang