Perhaps a better way would be to switch checksums globally rather than in each fetch call.

@dimitri-yatsenko What if we added a dj.config setting that takes a list of tables to disable checksum verification on? That way the code can remain the same though the config will persist but users won't just naively apply it everywhere. We can start ignoring it after a certain release (with a warning initially). That way we have a plan to enforce (again) the checksums once it has been properly optimized.

The main reason we had included this at the scope of each fetch is so the user will do a value assessment to determine if a particular fetch could really use the benefit of disabling checksums as opposed to a blanket application everywhere. This could still protect many other external fetches if the data is <10GiB. We will eventually upgrade the efficiency of checksums; it will just take some time to get it right so this is a stopgap since urgent improvement is necessary.

I agree that a global dj.config setting separate from safemode would be appropriate for checksum verification on fetch.

How about to summarize:

• We'll introduce a new dj.config key (disable_fetch_file_verification). It's value is a list of full table names i.e. [`schema`.`table`]
• This will only affect filepath@store attributes. This is easier since attachment and filepath checksums are handled in 2 different places (fetch.py:_get, external.py:download_filepath) and it will get messy. Trying not to refactor since we are thinking of a new solution to supersede external storage anyway.
• We'll need to add a new kwarg to download_filepath so that it knows the table name for the fetch that is triggering the download and we can handle it all in one place. Pass the table name associated with the file download to download_filepath so that it can perform the validation against the config.
• If this key exists and the table associated with the file download is in the list, then disable the checksum validation and throw a warning during the download step.

@dimitri-yatsenko Thumbs up if you agree and let me know if you want to catch up on this.

@guzman-raphael

1. Perhaps the setting should be global rather than doing this for each file. Or perhaps we could do this for specific schemas.
2. Can we use a positive formulation, e.g. disable_fetch_file_verification -> verify_file_checksum_on_fetch. Then statements with double negatives such as if not disable_fetch_file_verification: become a positive if verify_file_checksum_on_fetch.
3. I agree to only doing this for filepath. For attach, different files can have the same name, so checksum becomes a file identification mechanism, not just integrity check.

@dimitri-yatsenko

1. I disagree with this only because I would still discourage usage of disabling checksums. I'm normally in favor of simplicity but here it would cost us disabling this integrity check everywhere. There are probably only a handful of tables tracking such large raw data, right?

2. This would be nice but don't think it would be compatible with 1 as it would be cumbersome to explicitly list all tables. The check could be if table in disable_fetch_file_verification or checksum().

3. 👍

@dimitri-yatsenko and I met yesterday and discussed this. We converged on a simpler approach that could address the issue more directly.

To summarize:

• Add a new global config to dj.config that will specify a file size limit where checksums will only be evaluated for fetch of filepath files less than that size.
• Perhaps it could be called filepath_checksum_size_limit. It could take its value as an integer representing the bytes.
• None (default) would indicate no limit i.e. perform the checksum for all filepath fetches.
• 0 would be discouraged but could basically disable fetch checksums for all filepath.
• Throw a warning if a checksum is skipped due to this setting.
• We'd recommend that for users interested in using this stopgap (until further optimization is developed) to prevent verification, they should do this at their own risk and ideally tune it to a reasonable size that is affecting performance for their case.

Now relies on #1031 to be merged first. This is due to needing to use the new logger to test if a warning was thrown when disabling checksums.