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

Add additional debugging information #3682

Open
wants to merge 1 commit into
base: production
Choose a base branch
from

Conversation

grantfitzsimmons
Copy link
Member

@melton-jason This is based on the contents of your Slack message. Can you review and edit where necessary so that we can expand the information on this for the public?

The Specify backend also special behavior while in DEBUG mode:
- When the backend encounters an error, Django returns an error message containing comprehensive information of the error and potentially sensitive system information
- Specify validates the WorkBench upload plan when fetching the WorkBench's upload results
- From the `/api/workbench/upload_results/(?P<ds_id>\d+)/` endpoint
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can make this more human readable.
(?P<ds_id>\d+) just represents the dataset id you are fetching the results for.
For example, /api/workbench/upload_results/2/ is valid for this endpoint.
Not sure what would be the best way of formatting this.
Maybe some suggestions: /api/workbench/upload_results/DATASET_ID/ or /api/workbench/upload_results/<dataset_id>/

Comment on lines +346 to +347
- Specify adds an indent to the datamodel returned from the `context/datamodel.json` endpoint
- (DISABLED BY DEFAULT) Specify allows any endpoint/view to be profiled by adding a `/?prof` query parameter
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure how thorough you wish to be, but just having it adds an indent and formats the datamodel returned from the endpoint might just be taking up space.

Someone from @specify/dev-testing or @specify/developers will have to take a look at the profiling middleware and see if it's even functional when enabled.
Theoretically, it just needs to be enabled by removing the comment in specifyweb/settings/__init__.py

# 'middleware.profilemiddleware.ProfileMiddleware',

- Specify adds an indent to the datamodel returned from the `context/datamodel.json` endpoint
- (DISABLED BY DEFAULT) Specify allows any endpoint/view to be profiled by adding a `/?prof` query parameter

**Frontend:**
Copy link
Member

Choose a reason for hiding this comment

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

Note, enabling DEBUG mode using SP7_DEBUG does not affect front-end at all!

Front-end debug mode is enabled separately:

command: npx webpack -w --mode development --color

RUN npx webpack --mode production

(-w means watch for changes and rebuild front-end on changes)

See documentation at https://webpack.js.org/api/cli/

Copy link
Member

Choose a reason for hiding this comment

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

And unlike with python where debug mode can be enabled/disabled at will, front-end files are compiled from typescript into javascript and production/development mode must be specified during compilation.

In production, webpack is not running at all (it's only present at all when the docker image is being built by github action, and after that the build artifacts are copied into docker image, but not webpack itself as it's not needed anymore)

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for the clarification! I was trying to find where in the code the frontend handled setting the mode (which I know is through process.env.NODE_ENV ).

Good to know it's enabled with the command!

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