Add prefix to GCS and S3 uri separators

[tigrux]

Storage adapters for both GCS and S3 use a separator for their URIs. The symbols in each adapter use a prefix to avoid collisions. However, this is not the case for the symbol kSep. This change fixes such potential collision.

[netlify]

✅ Deploy Preview for meta-velox canceled.

Name	Link
🔨 Latest commit	6e34e5d
🔍 Latest deploy log	https://app.netlify.com/sites/meta-velox/deploys/65afed97cc748900081e9987

[tigrux]

Hello @majetideepak. We found an issue related to symbol collisions, and this contribution solves that problem. Thanks.

[facebook-github-bot]

@kgpai has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Need further clarifications.

[kgpai]

Hi @tigrux ,
Thank you for this change, However can you open an issue with the exact problem you are seeing ? Are you seeing issues during compile or link time ? Can you add these details to your issue ? Is this because of the anonymous namespacing ?

Thanks !

[majetideepak]

@kgpai I can see that users would see redefinition of 'kSep' compile-time error if they include both GCSUtil.h and S3Util.h

[tigrux]

Hello @kgpai, I filed the bug with exactly what @majetideepak described:
#8372

[facebook-github-bot]

@kgpai has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[kgpai]

Hi again @tigrux ,

Apologies for my late response on this, we are trying to move away from using anonymous namespaces in header files and I am going to create a PR shortly against our coding convention to highlight that. I do realize that this is present in more places than we would like in Velox, but we are trying to not add more.
Hopefully you can make this minor change and I will work on merging this.

[majetideepak]

we are trying to move away from using anonymous namespaces in header files

@kgpai Can you share some details on doing this? I am curious.

[tigrux]

Hi again @tigrux ,

Apologies for my late response on this, we are trying to move away from using anonymous namespaces in header files and I am going to create a PR shortly against our coding convention to highlight that. I do realize that this is present in more places than we would like in Velox, but we are trying to not add more. Hopefully you can make this minor change and I will work on merging this.

Hello, I understand. Should I then modify this PR to remove the anonymous namespaces (in S3 and GCS)?

[kgpai]

@tigrux Yes that would be great.
@majetideepak I will create a PR with more details soon.

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. If you'd still like this PR merged, please comment on the PR, make sure you've addressed reviewer comments, and rebase on the latest main. Thank you for your contributions!