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

Updated general exceptions with specific ones. #3339

Merged
merged 19 commits into from
Mar 14, 2023

Conversation

prernadabi23
Copy link
Contributor

@prernadabi23 prernadabi23 commented Mar 8, 2023

Added ValueError replacing Exception Error.

Signed-off-by: Prerna Dabi [email protected]

fixes #3337

Added ValueError replacing Exception Error.
suhaibmujahid
suhaibmujahid previously approved these changes Mar 8, 2023
Copy link
Member

@suhaibmujahid suhaibmujahid left a comment

Choose a reason for hiding this comment

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

Thank you, @prernadabi23

This PR fixes:
#3337

The goal of #3337 is to replace all cases not only in model.py. I will merge this PR for now, but it will not close the issue.

@suhaibmujahid
Copy link
Member

@prernadabi23 could you please edit the pull request title to something more descriptive?

@prernadabi23 prernadabi23 changed the title Updated Model.py fix: Updated general exceptions with specific ones. Mar 8, 2023
@prernadabi23
Copy link
Contributor Author

Thank you, I will update it in all files.

Copy link
Member

@suhaibmujahid suhaibmujahid left a comment

Choose a reason for hiding this comment

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

Thank you, I will update it in all files.

Thank you! Then, could you please link your pull request to the issue?

@suhaibmujahid suhaibmujahid self-requested a review March 8, 2023 15:08
@prernadabi23
Copy link
Contributor Author

Thank you, I will update it in all files.

Thank you! Then, could you please link your pull request to the issue?

I am not able to understand as I have already linked it.

@prernadabi23 prernadabi23 changed the title fix: Updated general exceptions with specific ones. #3337 fix: Updated general exceptions with specific ones. Mar 8, 2023
@suhaibmujahid
Copy link
Member

Thank you! Then, could you please link your pull request to the issue?

I am not able to understand as I have already linked it.

To allow GitHub to link the pull request to an issue, the keyword (i.e., "fixes") and the issue number should be in the same line without any special characters in between (e.g., fixes #3337).

See more here: https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue

@suhaibmujahid suhaibmujahid removed their request for review March 8, 2023 15:47
@prernadabi23 prernadabi23 changed the title #3337 fix: Updated general exceptions with specific ones. fix: Updated general exceptions with specific ones. Mar 8, 2023
@prernadabi23
Copy link
Contributor Author

@suhaibmujahid I mistakenly changed the PR name, and that has led to failing the lint check...
can you please look into this issue?

@prernadabi23 prernadabi23 changed the title fix: Updated general exceptions with specific ones. Updated general exceptions with specific ones. Mar 8, 2023
@suhaibmujahid
Copy link
Member

suhaibmujahid commented Mar 8, 2023

@suhaibmujahid I mistakenly changed the PR name, and that has led to failing the lint check... can you please look into this issue?

The lint error is not related to the PR name/title. The patch has a formatting problem, see: https://community-tc.services.mozilla.com/tasks/Ap5h1JETTveSymWIgrqscg/runs/0/logs/public/logs/live.log#L168-184

To avoid lint failures, it is recommended to setup pre-commit: https://github.com/mozilla/bugbug#auto-formatting

Since your changes have already been committed, once you have pre-commit installed, you can test all files using the following command:

pre-commit run --all-files

@prernadabi23
Copy link
Contributor Author

prernadabi23 commented Mar 9, 2023

@suhaibmujahid I did "pre-commit run --all-files". What else needs to be done? The error is still there.

@suhaibmujahid
Copy link
Member

@suhaibmujahid I did "pre-commit run --all-files". What else needs to be done? The error is still there.

You need to fix the lint errors and commit the fix.

@prernadabi23
Copy link
Contributor Author

prernadabi23 commented Mar 9, 2023

@suhaibmujahid I am getting a flake8 error. Else everything is passed.

$ pre-commit run --all-files
isort....................................................................Passed
black....................................................................Passed
prettier.................................................................Passed
flake8...................................................................Failed

  • hook id: flake8
  • exit code: 1

//////////////////////////////////////////////////////////////////////////////////////////

check python ast.........................................................Passed
check docstring is first.................................................Passed
check that executables have shebangs.....................................Passed
check for merge conflicts................................................Passed
check for broken symlinks............................(no files to check)Skipped
debug statements (python)................................................Passed
trim trailing whitespace.................................................Passed
check yaml...............................................................Passed
mixed line ending........................................................Passed
python tests naming......................................................Passed
check json...............................................................Passed
fix requirements.txt.....................................................Passed
check vcs permalinks.....................................................Passed
codespell................................................................Passed
taskcluster_yml..........................................................Passed
Strip unnecessary # noqas..............................................Passed
mypy-bugbug..............................................................Passed
mypy-bugbug-http.........................................................Passed
Check for useless excludes...............................................Passed

@prernadabi23
Copy link
Contributor Author

@suhaibmujahid It's all done.

bugbug/models/testselect.py Outdated Show resolved Hide resolved
bugbug/test_scheduling.py Outdated Show resolved Hide resolved
@prernadabi23
Copy link
Contributor Author

@suhaibmujahid you can check now.

bugbug/test_scheduling.py Outdated Show resolved Hide resolved
bugbug/test_scheduling.py Outdated Show resolved Hide resolved
infra/version_check.py Outdated Show resolved Hide resolved
@prernadabi23
Copy link
Contributor Author

@suhaibmujahid Can you confirm these changes, then I'll make the changes there.

@suhaibmujahid
Copy link
Member

@suhaibmujahid Can you confirm these changes, then I'll make the changes there.

You could commit the changes without asking 🙂

bugbug/repository.py Outdated Show resolved Hide resolved
bugbug/repository.py Outdated Show resolved Hide resolved
bugbug/rust_code_analysis_server.py Outdated Show resolved Hide resolved
scripts/commit_classifier.py Outdated Show resolved Hide resolved
http_service/tests/test_integration.py Outdated Show resolved Hide resolved
bugbug/repository.py Outdated Show resolved Hide resolved
bugbug/repository.py Outdated Show resolved Hide resolved
bugbug/repository.py Outdated Show resolved Hide resolved
bugbug/repository.py Outdated Show resolved Hide resolved
bugbug/repository.py Outdated Show resolved Hide resolved
@prernadabi23
Copy link
Contributor Author

Screenshot from 2023-03-15 00-52-56

@suhaibmujahid I don't know why the lint error is there, all checks are passing as you can see.

@prernadabi23
Copy link
Contributor Author

I am running two repos simultaneously, I'll fix this.

@prernadabi23
Copy link
Contributor Author

@suhaibmujahid Can you please look into this? I am trying but still no change.

Copy link
Member

@suhaibmujahid suhaibmujahid left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you, @prernadabi23!

Copy link
Collaborator

@marco-c marco-c left a comment

Choose a reason for hiding this comment

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

Thanks!

@marco-c marco-c merged commit 48a6d79 into mozilla:master Mar 14, 2023
@prernadabi23
Copy link
Contributor Author

@suhaibmujahid @marco-c Thank you so much ..! and @suhaibmujahid thank you for helping me in every step, learned a lot during this process.

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.

Replace general exceptions with a specific ones
3 participants