Use Sourcify database as default StorageService #1390
Conversation
services/server/src/server/controllers/repository/repository.handlers.ts
Outdated
Show resolved
Hide resolved
| "sources", | ||
| pathTranslationJSON[originalPath] | ||
| ) | ||
| (obj: any) => obj.path === relativeFilePath |
There was a problem hiding this comment.
I had to change this test because now a relative path is expected, not an absolute path
There was a problem hiding this comment.
I don't know what to do with this test. Both Database and RepositoryV2 don't store path translations, so testing it like this really doesn't make sense. Should I implement a new test to check specifically the files stored on RepositoryV1 or I simply get rid of it?
From my pov RepositoryV1 is going to be removed soon, we just need to find a way to run repo.sourcify.dev with RepositoryV2 or Database. After we remove it, testing path translations is going to be useless.
@kuzdogan @manuelwedler feedback?
There was a problem hiding this comment.
I'd say, in this case, don't make the extra effort for testing something that is going to be removed :)
There was a problem hiding this comment.
Actually looking at this test again... If we want to keep path sanitization, then it makes sense to keep this test. But do we want to keep path sanitization for the database?
There was a problem hiding this comment.
We can remove the path sanitization for the database but for the repoV2 this leaves me thinking if store the sources at path/to/the/source/with/many/folders/0xkeccak.sol or just sources/0xkeccak.sol because for the former there still can be weird characters in the path and we actually need to store in a standard path.
There was a problem hiding this comment.
RepositoryV2 stores everything inside the first level of /sources
e.g.
/sources/0x11111.sol
/sources/0x11112.sol
...
There was a problem hiding this comment.
Ok nice then it's not a problem and we can remove all the path sanitization related code + tests
There was a problem hiding this comment.
We cannot remove "all the path sanitization related code" because it's still used in RepositoryV1, and right now RepositoryV1 is used in repo.sourcify.dev
Should this PR also make repo.sourcify.dev point to RepositoryV2 and remove RepositoryV1 completely?
There was a problem hiding this comment.
Ok right... No let's not mix them both.
I've created a new issue to replace the repo.sourcify.dev completely and under that I've added the subtask to remove the path sanitization related code. #1398
With that we can dismiss this here and move on.
...ces/server/src/server/controllers/repository/get-contract-addresses-all.stateless.paths.yaml
Outdated
Show resolved
Hide resolved
...ces/server/src/server/controllers/repository/get-contract-addresses-all.stateless.paths.yaml
Outdated
Show resolved
Hide resolved
...ces/server/src/server/controllers/repository/get-contract-addresses-all.stateless.paths.yaml
Show resolved
Hide resolved
services/server/src/server/services/storageServices/SourcifyDatabaseService.ts
Outdated
Show resolved
Hide resolved
| "sources", | ||
| pathTranslationJSON[originalPath] | ||
| ) | ||
| (obj: any) => obj.path === relativeFilePath |
There was a problem hiding this comment.
We can remove the path sanitization for the database but for the repoV2 this leaves me thinking if store the sources at path/to/the/source/with/many/folders/0xkeccak.sol or just sources/0xkeccak.sol because for the former there still can be weird characters in the path and we actually need to store in a standard path.
…tabaseService.ts Co-authored-by: Kaan Uzdoğan <[email protected]>
…-addresses-all.stateless.paths.yaml Co-authored-by: Kaan Uzdoğan <[email protected]>
…-addresses-all.stateless.paths.yaml Co-authored-by: Kaan Uzdoğan <[email protected]>
This PR closes #1380
With this PR, all server endpoints that previously fetched information from RepositoryV1 will now retrieve data from the database and RepositoryV2.
This PR also sets the database as the source of truth when checking the existence of a match.
Todo
/files/contractsfor 200 contracts, implement new paginated APIs