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

Bower: disable strict-ssl #10370

Merged
merged 1 commit into from Jun 28, 2023
Merged

Bower: disable strict-ssl #10370

merged 1 commit into from Jun 28, 2023

Conversation

humitos
Copy link
Member

@humitos humitos commented May 31, 2023

I had to disable strict SSL on Bower due to an error with jQuery: bower/bower#2608

I'm not sure if this is strictly required, but I'm creating a PR in case we need to this in production as well.

Related #10356

I had to disable strict SSL on Bower due to an error with jQuery:
bower/bower#2608

I'm not sure if this is strictly required, but I'm creating a PR in case we need
to this in production as well.

Related #10356
@humitos humitos requested a review from a team May 31, 2023 13:10
@humitos humitos requested a review from a team as a code owner May 31, 2023 13:10
@benjaoming
Copy link
Contributor

Oh that seems annoying. Is this only for development and never for production?

There's an alternative to switching off SSL here that is popular: bower/bower#2608 (comment)

Copy link
Contributor

@agjohnson agjohnson left a comment

Choose a reason for hiding this comment

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

Not great, but we don't rebuild these assets often either way.

@humitos
Copy link
Member Author

humitos commented Jun 1, 2023

In theory, we rebuild them on each deploy when preparing the repositories.

@agjohnson
Copy link
Contributor

Ah hrm, yeah I forgot that was added to the release script. I guess we can see if it affects next build and can plan from there. I'm not too worried about this fix, and we need to continue building our assets.

@humitos
Copy link
Member Author

humitos commented Jun 6, 2023

I'm confused now. I just prepare the repositories, but I don't think the inv docker.buildassets command was called 🤷🏼

@benjaoming
Copy link
Contributor

@humitos I'm guessing that we only do builds with bower manually?

In that case, maybe this is fine? Perhaps post a follow-up issue to sort out if this old Node.js stack is staying or being replaced?

(assuming that the root of the issue is that most/all of the stack is outdated/deprecated, including some repo and its SSL certificate)

@humitos
Copy link
Member Author

humitos commented Jun 28, 2023

In that case, maybe this is fine? Perhaps post a follow-up issue to sort out if this old Node.js stack is staying or being replaced?

Yeah, "in theory", I understand the plan here "soonish" is to:

  • replace the nodejs stack here with the one from ext-theme
  • migrate all the readthedocs-embed.js feature to the new Addons js client

So, I think all the current stack is going to disappear in the medium term.

Copy link
Contributor

@benjaoming benjaoming left a comment

Choose a reason for hiding this comment

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

@humitos I think that a change like this which makes the build actually work is more desirable than the alternatives.

(I linked to an alternative that was using an unofficial repo which is also not a nice solution).

@humitos
Copy link
Member Author

humitos commented Jun 28, 2023

OK. I'm merging this for now so we unblock the generation of assets.

@humitos humitos merged commit 8d0b3d1 into main Jun 28, 2023
5 of 7 checks passed
@humitos humitos deleted the humitos/assets-ssl-issue branch June 28, 2023 13:41
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