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

Improve instructions for dev env #86

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

JasonGrace2282
Copy link
Member

@JasonGrace2282 JasonGrace2282 commented Sep 10, 2024

Just a bunch of patches and stuff I had to do to get local submissions & venv's working working.
It also improves the documentation, and adds a script or two.

@JasonGrace2282 JasonGrace2282 added the maintenance Dependencies, deprecation warnings, etc. label Sep 10, 2024
@JasonGrace2282 JasonGrace2282 self-assigned this Sep 10, 2024
@JasonGrace2282 JasonGrace2282 requested a review from a team as a code owner September 10, 2024 23:28
@JasonGrace2282 JasonGrace2282 changed the title Bugfixes to get a working dev env Improve instructions for dev env Sep 12, 2024
selsayed25
selsayed25 previously approved these changes Oct 7, 2024
Copy link
Member

@selsayed25 selsayed25 left a comment

Choose a reason for hiding this comment

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

The improvements to the documentation and setup scripts are very helpful. These changes look good to me!

docs/source/contributing/setup.rst Show resolved Hide resolved
scripts/create_wrappers.py Outdated Show resolved Hide resolved
Copy link
Member

@krishnans2006 krishnans2006 left a comment

Choose a reason for hiding this comment

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

Nice changes overall, but I'm a bit confused about some of them (specifically the switch to settings.USE_SANDBOXING).

@@ -273,5 +281,5 @@ def run_submission(submission_id):
submission.channel_group_name, {"type": "submission.updated"}
)

if os.path.exists(submission_wrapper_path):
with contextlib.suppress(FileNotFoundError):
Copy link
Member

Choose a reason for hiding this comment

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

Whats the reasoning behind this change?

Copy link
Member Author

Choose a reason for hiding this comment

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

Generally, with file IO stuff, checking if a file exists before deleting it is considered an antipattern (just because some other process can delete the file in between checking if it exists and deleting it). Usually, it's better to try deleting the file and ignore any problems that occur (like if the file doesn't exist).
This is also why we have the exist_ok parameters with stuff like os.path.mkdirs/Path.mkdir.

tin/apps/submissions/tasks.py Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Dependencies, deprecation warnings, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants