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

Job postings + ProductList extraction #103

Merged
merged 25 commits into from
Dec 16, 2024
Merged

Job postings + ProductList extraction #103

merged 25 commits into from
Dec 16, 2024

Conversation

kmike
Copy link
Contributor

@kmike kmike commented Dec 16, 2024

I merged it in a wrong branch initially :)

wRAR and others added 25 commits November 15, 2024 17:58
Extend the Search Queries description
It is useful as-is, but there are known issues - e.g. JS pagination
is common on job websites, and it's not handled by the spider, so on
many websites it can only extract jobs from the first page.
Job postings + ProductList extraction

def get_ephemeral_port():
s = socket.socket()
s.bind(("", 0))

Check warning

Code scanning / CodeQL

Binding a socket to all network interfaces Medium test

'' binds a socket to all interfaces.

Copilot Autofix AI about 1 month ago

To fix the problem, we need to bind the socket to a specific interface instead of all interfaces. This can be achieved by replacing the empty string ("") with a specific IP address. In this case, we will use 127.0.0.1 to bind the socket to the localhost interface, which limits access to the local machine only.

The changes will be made in the get_ephemeral_port function in the tests/mockserver.py file. Specifically, we will update the s.bind call on line 18 to use 127.0.0.1 instead of an empty string.

Suggested changeset 1
tests/mockserver.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/tests/mockserver.py b/tests/mockserver.py
--- a/tests/mockserver.py
+++ b/tests/mockserver.py
@@ -17,3 +17,3 @@
     s = socket.socket()
-    s.bind(("", 0))
+    s.bind(("127.0.0.1", 0))
     return s.getsockname()[1]
EOF
@@ -17,3 +17,3 @@
s = socket.socket()
s.bind(("", 0))
s.bind(("127.0.0.1", 0))
return s.getsockname()[1]
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not fixing it here; the intention of PR was to merge changes to the main branch.

Copy link

codecov bot commented Dec 16, 2024

Codecov Report

Attention: Patch coverage is 93.85475% with 11 lines in your changes missing coverage. Please review.

Project coverage is 95.36%. Comparing base (bacdae4) to head (8e040fa).
Report is 27 commits behind head on main.

Files with missing lines Patch % Lines
zyte_spider_templates/spiders/ecommerce.py 89.65% 6 Missing ⚠️
zyte_spider_templates/spiders/job_posting.py 95.69% 4 Missing ⚠️
zyte_spider_templates/spiders/base.py 93.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #103      +/-   ##
==========================================
- Coverage   95.39%   95.36%   -0.04%     
==========================================
  Files          25       26       +1     
  Lines        2454     2609     +155     
==========================================
+ Hits         2341     2488     +147     
- Misses        113      121       +8     
Files with missing lines Coverage Δ
zyte_spider_templates/__init__.py 100.00% <100.00%> (ø)
zyte_spider_templates/spiders/article.py 85.88% <100.00%> (+0.26%) ⬆️
zyte_spider_templates/spiders/serp.py 93.80% <100.00%> (+0.16%) ⬆️
zyte_spider_templates/spiders/base.py 86.20% <93.33%> (+2.48%) ⬆️
zyte_spider_templates/spiders/job_posting.py 95.69% <95.69%> (ø)
zyte_spider_templates/spiders/ecommerce.py 94.88% <89.65%> (-2.18%) ⬇️

... and 1 file with indirect coverage changes

@kmike kmike merged commit 5f5b59f into main Dec 16, 2024
12 of 13 checks passed
@wRAR wRAR deleted the articles_to_main branch December 18, 2024 08:28
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

Successfully merging this pull request may close these issues.

3 participants