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

Fix GCS bucket name validation #7227

Merged
merged 6 commits into from
Dec 13, 2023
Merged

Conversation

jootten
Copy link
Contributor

@jootten jootten commented Dec 5, 2023

Motivation and context

It is not possible to attach a GCS cloud storage bucket which name contains a dot. As stated here, GCS bucket names allow dots. This is a major problem for us, and this PR fixes this issue.

How has this been tested?

No tests required.

Checklist

  • I submit my changes into the develop branch
  • I have created a changelog fragment
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • I have linked related issues (see GitHub docs)
  • I have increased versions of npm packages if it is necessary
    (cvat-canvas,
    cvat-core,
    cvat-data and
    cvat-ui)

License

  • I submit my code changes under the same MIT License that covers the project.
    Feel free to contact the maintainers if that's a concern.

Sorry, something went wrong.

@jootten jootten changed the title Fix gcs bucket name validation Fix GCS bucket name validation Dec 5, 2023
@Marishka17
Copy link
Contributor

@jootten, Hello, thank you for the contribution!

Looks like it also affects attaching AWS S3 buckets. Could you please add . directly to COMMON_ALLOWED_RESOURCE_NAME_SYMBOLS?

@jootten
Copy link
Contributor Author

jootten commented Dec 7, 2023

@jootten, Hello, thank you for the contribution!

Looks like it also affects attaching AWS S3 buckets. Could you please add . directly to COMMON_ALLOWED_RESOURCE_NAME_SYMBOLS?

It looks like Azure does not support dots. I would suggest adding an explicit condition for AWS.

Copy link

codecov bot commented Dec 8, 2023

Codecov Report

Merging #7227 (7099a2a) into develop (560c17f) will increase coverage by 0.25%.
Report is 7 commits behind head on develop.
The diff coverage is 75.00%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #7227      +/-   ##
===========================================
+ Coverage    81.51%   81.77%   +0.25%     
===========================================
  Files          365      366       +1     
  Lines        39261    39355      +94     
  Branches      3631     3641      +10     
===========================================
+ Hits         32004    32182     +178     
+ Misses        7257     7173      -84     
Components Coverage Δ
cvat-ui 75.92% <ø> (+0.57%) ⬆️
cvat-server 87.09% <75.00%> (-0.01%) ⬇️

bsekachev and others added 2 commits December 8, 2023 13:30
Co-authored-by: Maria Khrustaleva <maria@cvat.ai>
Co-authored-by: Maria Khrustaleva <maria@cvat.ai>
@Marishka17 Marishka17 merged commit cbd5f43 into cvat-ai:develop Dec 13, 2023
33 checks passed
@cvat-bot cvat-bot bot mentioned this pull request Jan 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants