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

Sunbird CDN configuration #3594

Merged
merged 4 commits into from
Nov 25, 2022
Merged

Sunbird CDN configuration #3594

merged 4 commits into from
Nov 25, 2022

Conversation

surabhi-mahawar
Copy link
Contributor

@surabhi-mahawar surabhi-mahawar commented Nov 14, 2022

Jira ticket: SB-31158

Description: This PR includes the environment variables required for sunbird cloud agnostic tool. We have added conditions to use this tool or other services based on the configuration. This tool provides option to use AWS/Azure etc which will also be handled via sunbird confirguation variables SUNBIRD_CLOUD_MEDIA_STORAGE_TYPE.

We intend to store all the media files(Image/Audio/Video/Document) being used in UCI assessment bot. Also use this to display media files as well.

If these configuration is not set, it will start the service but will just show error message in the service logs and will not respond to that particular media related message.

@surabhi-mahawar
Copy link
Contributor Author

@keshavprasadms
Can you please merge this PR.

@beepdot
Copy link
Contributor

beepdot commented Nov 14, 2022

@surabhi-mahawar

  • Please add a detailed description on what this change is about
  • Jira link references
  • Documentation link if any
  • What do you intend to store in these buckets / containers
  • What would happen if CDN / media storage is not configured for an environment
  • What about other CSP's implementation

@surabhi-mahawar
Copy link
Contributor Author

@keshavprasadms
I have updated the description of the PR with jira ticket and the required information you mentioned.
We have added these environment variables for the sunbird's cloud agnostic tool only.

AZURE_BLOB_STORE_ACCOUNT_NAME={{sunbird_private_storage_account_name}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@surabhi-mahawar Are these variable still required? Below is another variable which has same contents
SUNBIRD_CLOUD_MEDIA_STORAGE_KEY

Copy link
Contributor Author

@surabhi-mahawar surabhi-mahawar Nov 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@keshavprasadms
No this is not required not, I have removed these variables from the files

@beepdot
Copy link
Contributor

beepdot commented Nov 16, 2022

@surabhi-mahawar This PR is supporting only Azure cloud right? In the description, I can see AWS support also, but I am unable to find the corresponding AWS cloud variables in the PR.

@surabhi-mahawar
Copy link
Contributor Author

@surabhi-mahawar This PR is supporting only Azure cloud right? In the description, I can see AWS support also, but I am unable to find the corresponding AWS cloud variables in the PR.

@keshavprasadms The sunbird cloud agnostic tool should handle this according to the SUNBIRD_CLOUD_MEDIA_STORAGE_TYPE mentioned, if this changes to AWS, the credentials for it should be added in SUNBIRD_CLOUD_MEDIA_STORAGE_KEY, SUNBIRD_CLOUD_MEDIA_STORAGE_SECRET as well.
It should change for everyone right, not just for one instance.
It should be either Azure or AWS.

Or can there be a case where different instances can have different CDNs ?

@beepdot
Copy link
Contributor

beepdot commented Nov 16, 2022

@surabhi-mahawar Please can you connect with @santhosh-tg for this? He is driving a change across all BB's to standardize the variable names and the naming convention, how to support multiple clouds etc. A brief discussion with him would help you get clarity and take this PR forward. Would be good if you can please setup a call and discuss with the team - @santhosh-tg @gandham-santhosh

@surabhi-mahawar
Copy link
Contributor Author

surabhi-mahawar commented Nov 18, 2022

@surabhi-mahawar Please can you connect with @santhosh-tg for this? He is driving a change across all BB's to standardize the variable names and the naming convention, how to support multiple clouds etc. A brief discussion with him would help you get clarity and take this PR forward. Would be good if you can please setup a call and discuss with the team - @santhosh-tg @gandham-santhosh

@keshavprasadms
Can you please send us the email IDs for @santhosh-tg @gandham-santhosh, so we can connect with them.

CC: @ChakshuGautam

@beepdot
Copy link
Contributor

beepdot commented Nov 18, 2022

I don't think adding email id's here is a good idea due to privacy concerns. As @ChakshuGautam is already on Slack, you can easily get in touch with the folks. These are the slack handlers

  • @Santhosh Gandham
  • @Santhosh Kumar Sreedharan

@surabhi-mahawar
Copy link
Contributor Author

@keshavprasadms
I have made the changes in the PR as per the variables mentioned in the sheet & discussion with Santhosh Kumar Sreedharan.
Can you please review this PR.

CC: @ChakshuGautam

@santhosh-tg santhosh-tg merged commit 2bc7760 into project-sunbird:release-5.1.0 Nov 25, 2022
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.

3 participants