add correctness check for bucket name

[kevincmchen]

Summary:

add correctness check for bucket name.

fix the todo in velox/connectors/hive/storage_adapters/s3fs/S3Util.h#L92

[netlify[bot]]

✅ Deploy Preview for meta-velox canceled.

Name	Link
🔨 Latest commit	eaad48a7039d0a447aa27a8a032553c2819ad09b
🔍 Latest deploy log	https://app.netlify.com/sites/meta-velox/deploys/66a3658af8743f00072c55fa

[kevincmchen]

@majetideepak @Yuhta can you take a look ？

[majetideepak]

@kevincmchen This looks better. But I wonder if the regex checks can get expensive when there are many files. Can you also clarify the current behavior if an invalid bucket is specified? Does the error from S3 indicate an invalid bucket? I am also curious as to when the bucket name can be invalid. In practice is this possible? Metastore and other metadata services should have valid bucket names since they are updated by the writers after successful writes to S3.

[kevincmchen]

Does the error from S3 indicate an invalid bucket?

it will not indicate a invalid bucket error instead of some error like bucket is empty/null.

But I wonder if the regex checks can get expensive when there are many files

I also think its expensive. in most cases, the bucket name is correct. doing correctness check for bucket is a little bit more than worth it.

So I don't think we need to do this check here。 and I just want to delete this todo, Is that OK with you? @majetideepak

[stale[bot]]

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!