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

feat: Implement error handling and return it to analytics #31

Merged

Conversation

mfteloglu
Copy link
Contributor

@mfteloglu mfteloglu commented Jan 10, 2024

Motivation

Changes

  • task_id is adapted to /companies endpoint
  • New custom errors are defined
  • Analytics is informed with /crawling-finished
  • task_id and error list is returned via /crawling-finished

Checklist

  • added myself as assignee
  • correct reviewers
  • descriptive PR title using conventional commits.
  • description explains the motivation and details of the changes
  • tests cover my changes
  • my functions are fully typed
  • documentation is updated
  • CI is green
  • breaking changes are discussed with the team and documented in the PR title ! (e.g. feat!: Update endpoint)

Copy link

linear bot commented Jan 10, 2024

DS-122 Implement logging and update Notion page

Please implement the logging for at lease one exemplary module, sending the data to the crawling finished endpoint in the Analytics endpoint. Please ping us when you are done so we can apply this for all other modules as well.

Also, please ensure the logging Notion page is up-to-date.

@mfteloglu mfteloglu self-assigned this Jan 10, 2024
@github-actions github-actions bot added the enhancement New feature or request label Jan 10, 2024
@mfteloglu mfteloglu marked this pull request as draft January 10, 2024 19:08
Copy link

github-actions bot commented Jan 10, 2024

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
235 153 65% 0% 🟢

New Files

File Coverage Status
parma_mining/github/helper.py 75% 🟢
parma_mining/mining_common/exceptions.py 89% 🟢
TOTAL 82% 🟢

Modified Files

File Coverage Status
parma_mining/github/analytics_client.py 45% 🟢
parma_mining/github/api/main.py 66% 🟢
parma_mining/github/client.py 33% 🟢
parma_mining/github/model.py 85% 🟢
TOTAL 57% 🟢

updated for commit: 8be8897 by action🐍

@mfteloglu mfteloglu changed the title feat: implement error handling and return it to analytics feat: Implement error handling and return it to analytics Jan 10, 2024
@mfteloglu mfteloglu marked this pull request as ready for review January 10, 2024 19:09
@mfteloglu mfteloglu marked this pull request as draft January 10, 2024 19:09
@mfteloglu mfteloglu changed the title feat: Implement error handling and return it to analytics feat: Implement error handling and return it to analytics Jan 10, 2024
@egekocabas
Copy link
Contributor

I fully tested these changes with the la-famiglia-jst2324/parma-analytics#54 and I had some fixes in analytics PR as well 😅
I am leaving for @Constantin343 's review for the implementation but it is working 👌🏻

Copy link
Contributor

@Constantin343 Constantin343 left a comment

Choose a reason for hiding this comment

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

Looks good! Only one suggestion for refactoring

@mfteloglu mfteloglu marked this pull request as ready for review January 12, 2024 01:40
@mfteloglu mfteloglu merged commit b9e12ad into main Jan 12, 2024
7 of 10 checks passed
@mfteloglu mfteloglu deleted the feature/ds-122-implement-logging-and-update-notion-page branch January 12, 2024 01:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants