Bump ruby 3 bump gems by jbat · Pull Request #60 · rdytech/superset-client

jbat · 2025-11-19T04:30:51Z

Loosens a number of dependencies

updates gemspec required Ruby required_ruby_version = ">= 3"
for local running via bin/console .. sets .ruby-version to 3.4.4
and for local running in docker, bump Dockerfile to use 3.4.4
loosens constraints on a number of gems to use '>=' over the more restrictive '~='
bump Version from 0.2.7 to 0.3.0

Copilot

Pull request overview

This PR upgrades the gem to require Ruby 3.x and loosens dependency version constraints. However, there are several critical issues that need to be addressed before merging:

Key Changes:

Updates minimum Ruby version requirement from 2.7.8 to 3.0
Loosens most gem dependency constraints from pessimistic (~>) to open-ended (>=)
Updates test expectations to match Ruby 3's hash inspection format changes

Reviewed changes

Copilot reviewed 9 out of 13 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
superset.gemspec	Updates required Ruby version to >= 3 and loosens dependency constraints
lib/superset/version.rb	Bumps version from 0.2.7 to 0.3.0
Dockerfile	Updates base image to Ruby 3.4.4
.ruby-version	Changes Ruby version from 2.7.8 to 3.4.4
.rubocop.yml	Updates target Ruby version from 2.6 to 3.0
README.md	Updates documentation to reflect Ruby >= 3.0.0 requirement
spec/*	Updates test expectations for Ruby 3 hash format (spaces around `=>`)
Gemfile.lock	Reflects updated dependencies from gemspec changes

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

lib/superset/version.rb

jbat · 2025-11-25T23:50:35Z

superset.gemspec

-  spec.add_dependency "rollbar", "~> 3.4"
-  spec.add_dependency "require_all", "~> 3.0"
-  spec.add_dependency "rubyzip", "~> 1.0"
+  spec.add_dependency "rake", ">= 13.0"


I have no issue with leaving rake >= 13

Copilot · 2025-11-25T22:59:17Z

superset.gemspec

+  spec.add_dependency "rake", ">= 13.0"
+  spec.add_dependency "rollbar", ">= 3.0"
+  spec.add_dependency "require_all", ">= 3.0"
+  spec.add_dependency "rubyzip", ">= 1.0"


Using >= 1.0 for rubyzip is overly permissive. Rubyzip has had security vulnerabilities in the past, and allowing any version >= 1.0 could introduce known security issues or breaking changes. Consider using ~> 2.0 or at minimum >= 2.0 to avoid older versions with potential security vulnerabilities.

Suggested change

spec.add_dependency "rubyzip", ">= 1.0"

spec.add_dependency "rubyzip", "~> 2.0"

Copilot · 2025-11-25T22:59:18Z

superset.gemspec

-  spec.add_dependency "dotenv", "~> 2.7"
-  spec.add_dependency "json", "~> 2.6"
+  spec.add_dependency "dotenv", ">= 2.0"
+  spec.add_dependency "json", "~> 2.0"


The json dependency constraint was changed from ~> 2.6 to ~> 2.0, which is inconsistent with the stated goal of loosening constraints. This actually broadens the range of allowed versions (allowing older 2.x versions), but still uses the pessimistic constraint ~>. Consider whether this should be >= 2.0 to match the pattern of other loosened dependencies, or if ~> 2.0 is intentionally kept more restrictive.

Suggested change

spec.add_dependency "json", "~> 2.0"

spec.add_dependency "json", ">= 2.0"

Copilot · 2025-11-25T22:59:18Z

README.md

+## Setup Locally ( no docker )

-Requires Ruby >= 2.6.0 
+Requires Ruby >= 3.0.0


The documentation states "Requires Ruby >= 3.0.0" but the gemspec specifies required_ruby_version = ">= 3" which is semantically equivalent to >= 3.0.0. However, for consistency and clarity, consider updating the documentation to match the exact format used in the gemspec (">= 3") or vice versa.

Suggested change

Requires Ruby >= 3.0.0

Requires Ruby >= 3

Copilot · 2025-11-25T22:59:18Z

spec/superset/dataset/list_spec.rb

-          ["2", "birth_names", "{\"database_name\"=>\"examples\", \"id\"=>1}", "public", "bob"],
-          ["3", "birth_days", "{\"database_name\"=>\"examples\", \"id\"=>1}", "public", "bob"]
+          ["2", "birth_names", "{\"database_name\" => \"examples\", \"id\" => 1}", "public", "bob"],
+          ["3", "birth_days",  "{\"database_name\" => \"examples\", \"id\" => 1}", "public", "bob"]


There's inconsistent spacing in the array elements - line 58 has one space after the comma between elements, while line 59 has two spaces after "birth_days". For consistency, both lines should have the same spacing pattern.

Suggested change

["3", "birth_days", "{\"database_name\" => \"examples\", \"id\" => 1}", "public", "bob"]

["3", "birth_days", "{\"database_name\" => \"examples\", \"id\" => 1}", "public", "bob"]

superset.gemspec

-  spec.add_dependency "require_all", "~> 3.0"
-  spec.add_dependency "rubyzip", "~> 1.0"
+  spec.add_dependency "rake", ">= 13.0"
+  spec.add_dependency "rollbar", ">= 3.0"


superset.gemspec

-  spec.add_development_dependency "rubocop", "~> 1.5"
-  spec.add_development_dependency "pry", "~> 0.14"
+  spec.add_development_dependency "rspec", ">= 3.0"
+  spec.add_development_dependency "rubocop", ">= 1.0"


schlick · 2025-11-26T02:20:53Z

superset.gemspec

  # Uncomment to register a new dependency of your gem
-  spec.add_dependency "dotenv", "~> 2.7"
-  spec.add_dependency "json", "~> 2.6"
+  spec.add_dependency "dotenv", ">= 2.0"


Can we make dotenv a development dependency instead?

schlick · 2025-11-26T02:21:29Z

superset.gemspec

-  spec.add_dependency "rollbar", "~> 3.4"
-  spec.add_dependency "require_all", "~> 3.0"
-  spec.add_dependency "rubyzip", "~> 1.0"
+  spec.add_dependency "rake", ">= 13.0"


Can we make rake a development dependency instead?

schlick · 2025-11-26T02:24:29Z

superset.gemspec

-  spec.add_dependency "require_all", "~> 3.0"
-  spec.add_dependency "rubyzip", "~> 1.0"
+  spec.add_dependency "rake", ">= 13.0"
+  spec.add_dependency "rollbar", ">= 3.0"


Can we make rollbar a development dependency instead? It would require changing its usage to be something like:

if defined?(Rollbar) Rollbar.error("Warm up cache failed for the dashboard #{dashboard_id.to_s} and for the dataset #{dataset["datasource_name"]} - #{e}")

This way the gem is not forcing all consumers to use rollbar (even though we are).

Makes sense .. I was thinking of pulling it into some configurable exception handling service class
but this is good enough for now.

schlick · 2025-11-26T02:24:56Z

superset.gemspec

-  spec.add_dependency "rubyzip", "~> 1.0"
+  spec.add_dependency "rake", ">= 13.0"
+  spec.add_dependency "rollbar", ">= 3.0"
+  spec.add_dependency "require_all", ">= 3.0"


Does the require_all gem really provide much value?

I believe it does, yes.
If we remove it, there would be around 70 classes to explicitly require.

require_rel "superset" recursively loads all 70 files in lib/superset/ (including subdirectories like chart/, dashboard/, dataset/, database/, security/, services/, tag/, sqllab/, enumerations/)

schlick · 2025-11-26T02:35:16Z

superset.gemspec

+  spec.add_dependency "rollbar", ">= 3.0"
+  spec.add_dependency "require_all", ">= 3.0"
+  spec.add_dependency "rubyzip", ">= 1.3"
  spec.add_dependency "faraday", "~> 1.0"


Add some point we'll need to look into migrating our apps to faraday v2

Agreed .. some work on Happi gem started on that a few months ago, wip
https://github.com/rdytech/happi/releases. rc1

and
rdytech/happi#21 Rc2

jbat added 3 commits November 19, 2025 15:39

Upgrade to ruby >=3, update specs

8803916

further loosen dependencies

0f73cac

bump version to 3.0.0

3c7f72a

jbat force-pushed the bump-ruby-3-bump-gems branch from 2efebf6 to 3c7f72a Compare November 19, 2025 04:40

jbat added 2 commits November 19, 2025 15:46

update Gemfile.lock, fix spec

03adaf3

up Dockerfile to ruby 3.4.4

529f5da

jbat requested a review from Copilot November 25, 2025 22:56

Copilot started reviewing on behalf of jbat November 25, 2025 22:56 View session

Copilot finished reviewing on behalf of jbat November 25, 2025 22:58

Copilot AI reviewed Nov 25, 2025

View reviewed changes

update as per pilot review

2ba8d82

schlick reviewed Nov 26, 2025

View reviewed changes

jbat added 2 commits November 26, 2025 14:12

moved rake,dotenv to dev dependencies

d6ffc1b

move gems to dev dependencies

7d16316

schlick approved these changes Nov 26, 2025

View reviewed changes

jbat merged commit 6abd22d into develop Nov 26, 2025
1 check passed

	spec.add_dependency "rubyzip", ">= 1.0"
	spec.add_dependency "rubyzip", "~> 2.0"

	spec.add_dependency "json", "~> 2.0"
	spec.add_dependency "json", ">= 2.0"

	["3", "birth_days", "{\"database_name\" => \"examples\", \"id\" => 1}", "public", "bob"]
	["3", "birth_days", "{\"database_name\" => \"examples\", \"id\" => 1}", "public", "bob"]

Conversation

jbat commented Nov 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

jbat commented Nov 19, 2025 •

edited

Loading