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

PSVAMB-10947 -> CR Change (The upgrade process cannot require any int… #9428

Merged
merged 2 commits into from
Jun 3, 2020

Conversation

TalDubovKaltura
Copy link
Contributor

@TalDubovKaltura TalDubovKaltura commented May 20, 2020

@jessp01


This change is Reviewable

@jessp01
Copy link

jessp01 commented May 20, 2020

Hi @TalDubovKaltura ,

It's better, however:
a. I don't see that you are setting (or prompting, for that matter) for the -2 admin secret), to wit:

     <session>
        <partnerId>-2</partnerId>
        <secret>@ADMIN_CONSOLE_PARTNER_ADMIN_SECRET@</secret>
        <sessionType>2</sessionType>
    </session>

See deployment/updates/scripts/xml/2018_04_27_mediaspaceNotificationsTemplates.template.xml for example
b. When you are done and have tested, I need this pushed to 16.2.0 as well since I need to release it.

@jessp01
Copy link

jessp01 commented May 20, 2020

Also, all of these have the very same issue:

 # grep "prompt" /opt/kaltura/app/deployment/updates/ -r
/opt/kaltura/app/deployment/updates/scripts/xml/2020_03_30_User_Replied_To_Comment_AppSpecific.xml:        <serviceUrl>http://{prompt:Host name:}/</serviceUrl>
/opt/kaltura/app/deployment/updates/scripts/xml/2020_03_30_User_Deleted_A_Comment_AppSpecific.template.xml:        <serviceUrl>http://{prompt:Host name:}/</serviceUrl>
/opt/kaltura/app/deployment/updates/scripts/xml/2020_03_30_User_Replied_To_Comment_AppSpecific.template.xml:        <serviceUrl>http://{prompt:Host name:}/</serviceUrl>
/opt/kaltura/app/deployment/updates/scripts/xml/2020_03_30_User_Deleted_A_Comment_AppSpecific.xml:        <serviceUrl>http://{prompt:Host name:}/</serviceUrl>
/opt/kaltura/app/deployment/updates/scripts/xml/2020_03_30_User_Replied_To_Comment.template.xml:        <serviceUrl>http://{prompt:Host name:}/</serviceUrl>
/opt/kaltura/app/deployment/updates/scripts/xml/2020_03_30_User_Deleted_A_Comment.xml:        <serviceUrl>http://{prompt:Host name:}/</serviceUrl>
/opt/kaltura/app/deployment/updates/scripts/xml/2020_03_30_User_Replied_To_Comment.xml.backup:        <serviceUrl>http://{prompt:Host name:}/</serviceUrl>
/opt/kaltura/app/deployment/updates/scripts/xml/2020_03_30_User_Replied_To_Comment_AppSpecific.xml.backup:        <serviceUrl>http://{prompt:Host name:}/</serviceUrl>
/opt/kaltura/app/deployment/updates/scripts/xml/2020_03_30_User_Replied_To_Comment.xml:        <serviceUrl>http://{prompt:Host name:}/</serviceUrl>
/opt/kaltura/app/deployment/updates/scripts/xml/2020_03_30_User_Deleted_A_Comment.xml.backup:        <serviceUrl>http://{prompt:Host name:}/</serviceUrl>
/opt/kaltura/app/deployment/updates/scripts/xml/2020_03_30_User_Deleted_A_Comment_AppSpecific.xml.backup:        <serviceUrl>http://{prompt:Host name:}/</serviceUrl>
/opt/kaltura/app/deployment/updates/scripts/xml/2020_03_30_User_Deleted_A_Comment.template.xml:        <serviceUrl>http://{prompt:Host name:}/</serviceUrl>

…ocess cannot require any interactive inputs whatsoever)
@TalDubovKaltura
Copy link
Contributor Author

@jessp01
Secret has been added: c4fbc7f

As for the other files, where do you these without the '@SERVICE_URL@'?

After approving this pull request I will push it also to 16.2.0.
Thank you Jess

@TalDubovKaltura
Copy link
Contributor Author

I see them in all the files I noted above. For example: https://github.com/kaltura/server/blob/Propus-16.3.0/deployment/updates/script/xml/2020_03_30_User_Replied_To_Comment_AppSpecific.template.xml#L4
https://github.com/kaltura/server/blob/Propus-16.3.0/deployment/updates/scripts/xml/2020_03_30_User_Deleted_A_Comment.template.xml#L4
https://github.com/kaltura/server/blob/Propus-16.3.0/deployment/updates/scripts/xml/2020_03_30_User_Replied_To_Comment_AppSpecific.template.xml

The change you made seems fine, Just run the grep from my comment in your clone and fix every such instance.

@jessp01 I already ran it before and no file left with that, the links you attached is from 16.3.0 which still not includes the changes I added

@jessp01
Copy link

jessp01 commented May 21, 2020

Yes. You can merge and push to 16.2.0 as well.

@TalDubovKaltura
Copy link
Contributor Author

@jessp01 We don't need any permission from the BE Core? Can I push it for both branches? and I know we don't need here any deployment.

@jessp01
Copy link

jessp01 commented May 21, 2020

You can check with Moshe Maor. I don't see why he should object but in any case, I must have it in 16.2.0 as I need to release the version.

@jessp01
Copy link

jessp01 commented May 21, 2020

Any news?

@TalDubovKaltura
Copy link
Contributor Author

@jessp01 I added just now the changes here to 16.2.0 with cherry-pick. as for 16.3.0 pull request, we will wait for Moshe to respond

@jessp01
Copy link

jessp01 commented May 21, 2020

Building a new package and testing.

@jessp01
Copy link

jessp01 commented May 21, 2020

All good on 16.2.0. Please be sure to push this to 16.3.0.

@jessp01
Copy link

jessp01 commented May 25, 2020

@TalDubovKaltura,
What's our status on this one? It needs to be merged into 16.3.0

@TalDubovKaltura
Copy link
Contributor Author

@MosheMaorKaltura Can you confirm these changes?

@jessp01
Copy link

jessp01 commented Jun 1, 2020

@TalDubovKaltura , @MosheMaorKaltura ,
How many more times am I expected to ask about this?
We've gone through a full dev cycle since I first reported it. I cannot release 16.3.0 until this is fixed.

@jessp01
Copy link

jessp01 commented Jun 1, 2020

@zoharbabin - FYI.

@MosheMaorKaltura
Copy link
Contributor

Reviewed
Notice to merge to the correct branch 16.4

@jessp01
Copy link

jessp01 commented Jun 2, 2020

I need it merged to both. 16.3.0 cannot be released until this is merged to it.

@TalDubovKaltura TalDubovKaltura merged commit c8b9904 into Propus-16.3.0 Jun 3, 2020
@TalDubovKaltura
Copy link
Contributor Author

@jessp01 merged to both 16.3 and 16.4, please approve.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants