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

SAMZA-1582 : removed the yajl-ruby dependency for security reasons #486

Closed
wants to merge 1 commit into from

Conversation

cameronlee314
Copy link
Contributor

@cameronlee314 cameronlee314 commented Apr 24, 2018

The original ticket suggested updating the yajl-ruby dependency for security reasons.
Many of the ruby dependencies for building the docs site are years old. After upgrading the dependencies, the yajl-ruby dependency just went away.
The dependencies were upgraded to the highest version that was compatible with ruby 2.0.0 (jekyll was the only dependency which has newer versions that are only compatible with ruby versions greater than 2.0.0).

Testing done:
bundle exec jekyll serve --watch --baseurl ""
_docs/local-site-test.sh
sanity checked a couple of links

@cameronlee314 cameronlee314 changed the title SAMZA-1582 : Updated yajl-ruby ~> 1.3.1; actually just removed the ya… SAMZA-1582 : Updated yajl-ruby ~> 1.3.1; actually just removed the yajl-ruby dependency Apr 24, 2018
@vjagadish1989
Copy link
Contributor

LGTM. Were some of the dependencies unsupported/EOL'ed?
nit: update the PR title.

@cameronlee314
Copy link
Contributor Author

The dependencies weren't EOL'ed, but there was a transitive dependency on yajl-ruby, and the version Samza picked up had a security issue according to apache. I guess the newer versions of the direct Samza dependencies just stopped using yajl-ruby.
What would be the preferred format for the PR title? The suggested format I am aware of is " : ", so should I stick with just that, or make it a bit clearer what actually got changed?

Copy link
Contributor

@vjagadish vjagadish left a comment

Choose a reason for hiding this comment

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

lgtm thanks Cameron!

@cameronlee314 cameronlee314 changed the title SAMZA-1582 : Updated yajl-ruby ~> 1.3.1; actually just removed the yajl-ruby dependency SAMZA-1582 : removed the yajl-ruby dependency for security reasons Apr 25, 2018
@asfgit asfgit closed this in cf4872e Apr 27, 2018
@vjagadish
Copy link
Contributor

Merged and submitted! Thanks @cameronlee314. Please resolve/close the JIRA with the fix version

@cameronlee314 cameronlee314 deleted the yajlruby branch October 4, 2019 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants