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 codespell config and fix typos to tox -e lint, fix typos, add custom dictionary (not enabling for now) #1466

Merged
merged 5 commits into from
Feb 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .codespell_dict
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
zar->zarr
6 changes: 6 additions & 0 deletions .codespellrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[codespell]
skip = .git,node_modules,venvs,.tox,yarn.lock
# Disabled until https://github.com/codespell-project/codespell/issues/2727
# got answer/re-solution
# dictionary = .codespell_dict
# ignore-words-list =
2 changes: 1 addition & 1 deletion dandiapi/api/models/oauth.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def clean(self):
try:
super().clean()
except ValidationError as e:
# don't validate URLs so we can use wilcards too
# don't validate URLs so we can use wildcards too
if 'Enter a valid URL.' not in str(e):
raise e

Expand Down
2 changes: 1 addition & 1 deletion dandiapi/api/tests/test_asset.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ def test_asset_total_size(

assert Asset.total_size() == asset_blob.size + zarr_archive.size

# TODO: add testing for embargoed zar added, whenever embargoed zarrs
# TODO: add testing for embargoed zarr added, whenever embargoed zarrs
# supported, ATM they are not and tested by test_zarr_rest_create_embargoed_dandiset


Expand Down
4 changes: 2 additions & 2 deletions dandiapi/api/tests/test_asset_paths.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ def test_asset_path_add_version_asset_paths(draft_version_factory, asset_factory
version.assets.add(asset_factory(path='foo/baz/file.txt'))
version.assets.add(asset_factory(path='top.txt'))

# Add verison asset paths
# Add version asset paths
add_version_asset_paths(version)

# Check paths have expected file count and size
Expand Down Expand Up @@ -139,7 +139,7 @@ def test_asset_path_add_version_asset_paths_idempotent(draft_version_factory, as
version.assets.add(asset_factory(path='foo/baz/file.txt'))
version.assets.add(asset_factory(path='top.txt'))

# Add verison asset paths
# Add version asset paths
add_version_asset_paths(version)
add_version_asset_paths(version)

Expand Down
2 changes: 1 addition & 1 deletion dandiapi/api/tests/test_upload.py
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ def test_upload_initialize_embargo_existing_asset_blob(
dandiset = dandiset_factory(embargo_status=Dandiset.EmbargoStatus.EMBARGOED)
assign_perm('owner', user, dandiset)

# Embargoed assets that are already uploaded publically don't need to be private
# Embargoed assets that are already uploaded publicly don't need to be private
resp = api_client.post(
'/api/uploads/initialize/',
{
Expand Down
2 changes: 1 addition & 1 deletion dandiapi/api/views/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ class Meta:


class DandisetVersionSerializer(serializers.ModelSerializer):
"""The verison serializer nested within the Dandiset Serializer."""
"""The version serializer nested within the Dandiset Serializer."""

class Meta:
model = Version
Expand Down
2 changes: 1 addition & 1 deletion dandiapi/zarr/tests/test_ingest_zarr_archive.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ def test_ingest_zarr_archive_force(zarr_upload_file_factory, zarr_archive_factor
# Perform initial ingest
ingest_zarr_archive(str(zarr.zarr_id))

# Get inital checksum
# Get initial checksum
zarr.refresh_from_db()
first_checksum = zarr.checksum

Expand Down
4 changes: 2 additions & 2 deletions doc/design/deployment-1.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@ This proposed solution is to use a second `release` branch which tracks the rele
There are two central git branches:

- **`master`**: the active development branch. PRs should always use `master` as their merge target.
- **`release`**: the *current* release branch. This will be reset to point to the top of master whenever a release ocurrs.
- **`release`**: the *current* release branch. This will be reset to point to the top of master whenever a release occurs.

Staging is deployed from `master`, while production is deployed from `release`.

## The `release` branch
The `release` branch is kept up to date using a GitHub CI workflow. Whenever a release ocurrs, the `release` branch is reset to point to `master` (to avoid merge conflicts). The `release` branch should therefore always be pointed at the latest release tag.
The `release` branch is kept up to date using a GitHub CI workflow. Whenever a release occurs, the `release` branch is reset to point to `master` (to avoid merge conflicts). The `release` branch should therefore always be pointed at the latest release tag.

## Netlify deployment
The staging and production Netlify sites are now both managed using a single `netlify.toml`. [Deploy contexts](https://docs.netlify.com/configure-builds/file-based-configuration/#deploy-contexts) allow us to differentiate between the production and staging sites. Production uses the default configuration, while staging uses a `branch-deploy` configuration.
Expand Down
4 changes: 2 additions & 2 deletions doc/design/embargo-full.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ A link to the dandiset with this secret URL parameter is included somewhere on t
### TODO how to download anonymously with the CLI?

## Embargo period enforcement
NIH embargoes (and embargos in general) will have an end date to ensure that the data is not secret forever.
NIH embargoes (and embargoes in general) will have an end date to ensure that the data is not secret forever.
We will enforce that an end date be specified for every new embargoed dandiset, and forcibly release embargoed dandisets that expire.

The MVP collects the NIH award number and stores it in the metadata.
Expand All @@ -76,7 +76,7 @@ If this becomes an issue, we could:
This could be as simple as adding a new django app and a new Heroku dyno, or as complex as a Lambda@Edge+CloudFront service.
* Dynamically provision IAM users with permission to access prefixes in the embargo bucket and distribute access keys to users.
This would require the API server to manage IAM directly, which is a lot of complexity to manage.
* Make the embargo bucket publically readable, but not listable.
* Make the embargo bucket publicly readable, but not listable.
If anyone knows the full S3 object key they have the ability to download the data, but they will not have the ability to search for or scan the bucket for new content.
We would then distribute the zarr_id to anyone who needs to access an embargoed zarr archive, giving them permanent read access to it.
The downside is that access is not revocable, since we cannot take back the zarr ID from the user or efficiently change the location of the zarr archive.
Expand Down
6 changes: 3 additions & 3 deletions doc/design/embargo-mvp.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ Uploads to an embargoed dandiset will function exactly the same from an API pers

# Data storage
Embargoed assets will be stored in a separate S3 bucket.
This bucket is private and not browseable by the general public.
This bucket is private and not browsable by the general public.

Each dandiset stored in the embargoed bucket will be prefixed with a dandiset identifier.
This will make it easier to manage embargo permissions for a specific embargoed dandiset.
Expand All @@ -40,7 +40,7 @@ Assuming dandiset `123456` was embargoed:

When unembargoing an embargoed dandiset, all asset data for that dandiset is copied to the public bucket.

When uploading a new asset to an embargoed dandiset, the server will first check if that blob has already been uploaded publically.
When uploading a new asset to an embargoed dandiset, the server will first check if that blob has already been uploaded publicly.
If so, the public blob will be used instead of uploading the data again to the embargo bucket.

# Data download
Expand All @@ -62,7 +62,7 @@ A test implementation can be found [here](https://github.com/dandi/dandi-api/com

## Models
The `Dandiset` model will have an `embargo_status` field that is one of `EMBARGOED`, `UNEMBARGOING`, or `OPEN`.
* `OPEN` means that the Dandiset is publically accessible and publishable.
* `OPEN` means that the Dandiset is publicly accessible and publishable.
This is the state all Dandisets currently have.
* `EMBARGOED` means that the Dandiset is embargoed.
It is searchable and viewable to owners.
Expand Down
2 changes: 1 addition & 1 deletion doc/design/publish-1.md
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ If a Version is not `VALID` or any of the Assets in the Version is not `VALID`,
1. The dandiset is locked so that no other publishes can happen simultaneously.

2. A new published Version is created with the required publish metadata information.
It is initialy empty.
It is initially empty.

3. A new DOI is created that points to the new Version.
This is an API call to an external service, https://datacite.org/.
Expand Down
6 changes: 3 additions & 3 deletions doc/design/zarr-performance-redesign.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,11 @@ The process is as follows.

(Steps 1 and 2): `dandi-cli` asks the server to create a new Zarr archive, which is put into the `PENDING` state.

(Steps 3 and 4): For each batch of (maxiumum) 255 Zarr chunk files the client wants to upload, `dandi-cli` asks the server to create an `Upload`, supplying the list of file paths and associated etags, and receiving a list of signed upload URLs.
(Steps 3 and 4): For each batch of (maximum) 255 Zarr chunk files the client wants to upload, `dandi-cli` asks the server to create an `Upload`, supplying the list of file paths and associated etags, and receiving a list of signed upload URLs.

(Step 5): `dandi-cli` uses these URLs to upload the files in that batch.

(Steps 6 and 7): Then, `dandi-cli` asks the server to finalize the batch, and the server does so, matching etags and verifiying that all files were uploaded. *This step is very costly, due to the server's need to contact S3 to verify these conditions.*
(Steps 6 and 7): Then, `dandi-cli` asks the server to finalize the batch, and the server does so, matching etags and verifying that all files were uploaded. *This step is very costly, due to the server's need to contact S3 to verify these conditions.*

(Step 8): When all batches are uploaded, `dandi-cli` signals the server to ingest the Zarr archive.

Expand Down Expand Up @@ -208,4 +208,4 @@ and gain significant performance for Zarr upload.

We previously included extra functionality, which involved *including* the locally computed checksum when finalizing the zarr archive (step 7), and adding a `MISMATCH` state to the zarr `status` field, which would be set if the checksum produced by the asynchronous zarr checksum task didn't match the checksum provided in step 7.

This addition was later reverted in the interest of simplicity, as well as the fact that it is funtionally equivalent to the current design.
This addition was later reverted in the interest of simplicity, as well as the fact that it is functionally equivalent to the current design.
2 changes: 1 addition & 1 deletion doc/design/zarr-support-3.md
Original file line number Diff line number Diff line change
Expand Up @@ -152,4 +152,4 @@ This ensures that published dandisets are truly immutable.

Immutability is enforced by disabled the upload and delete endpoints for the zarr archive.

The client needs to agressively inform users that publishing a dandiset with a zarr archive will render that zarr archive immutable.
The client needs to aggressively inform users that publishing a dandiset with a zarr archive will render that zarr archive immutable.
3 changes: 3 additions & 0 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,11 @@ envlist =
skipsdist = true
extras =
lint
deps =
codespell~=2.0
commands =
flake8 --config=tox.ini {posargs:.}
codespell

[testenv:type]
extras =
Expand Down
4 changes: 2 additions & 2 deletions web/src/components/Meditor/state.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import Vue, { computed, ref } from 'vue';
import { EditorInterface } from './editor';

// NOTE: it would be better to use a single ref here instead of seperate state/computed
// NOTE: it would be better to use a single ref here instead of separate state/computed
// variables, but doing so introduces a strange bug where editorInterface.basicModel is
// un-reffed immediately after instantiation. This does not occur when using a computed
// variable with a seperate state object, so we do that here as a workaround.
// variable with a separate state object, so we do that here as a workaround.
const state = {
editorInterface: null as EditorInterface | null,
};
Expand Down
2 changes: 1 addition & 1 deletion web/src/rest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ const dandiRest = new Vue({
// Fetch user
this.user = await this.me();
} catch (e) {
// A status of 401 indicates login failed, so the exception should be supressed.
// A status of 401 indicates login failed, so the exception should be suppressed.
if (axios.isAxiosError(e) && e.response?.status === 401) {
await oauthClient.logout();
} else {
Expand Down