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

Upgrade to geoblacklight 4.0 #963

Merged
merged 18 commits into from
Jul 2, 2024
Merged

Upgrade to geoblacklight 4.0 #963

merged 18 commits into from
Jul 2, 2024

Conversation

jcoyne
Copy link
Contributor

@jcoyne jcoyne commented Mar 29, 2023

Blocked by changing all our metadata to the new schema used by v4: https://opengeometadata.org/docs/about-ogm-aardvark

@thatbudakguy
Copy link
Member

thatbudakguy commented Mar 29, 2024

Heads up!

This branch (and not main) is currently connected to continuous deployment to -stage and -UAT while we experiment with new indexing config (sul-dlss/searchworks_traject_indexer#770).

Once the indexing is solid and the new production core is populated with Aardvark-format records, it can be merged and CD can be recoupled to main.

To do that, you can just revert 1602a5e.

@thatbudakguy thatbudakguy force-pushed the geobl4 branch 5 times, most recently from 188272b to 5f5c53b Compare June 5, 2024 22:03
@thatbudakguy thatbudakguy changed the title Upgrade to geoblacklight 4.0 [HOLD] Upgrade to geoblacklight 4.0 Jun 5, 2024
@thatbudakguy thatbudakguy force-pushed the geobl4 branch 2 times, most recently from c9c74be to 064dcc9 Compare June 12, 2024 21:32
@thatbudakguy thatbudakguy marked this pull request as ready for review June 13, 2024 21:04
@thatbudakguy thatbudakguy changed the title [HOLD] Upgrade to geoblacklight 4.0 Upgrade to geoblacklight 4.0 Jun 28, 2024
@thatbudakguy thatbudakguy force-pushed the geobl4 branch 4 times, most recently from 043e237 to 2fe797f Compare June 28, 2024 18:19
@thatbudakguy thatbudakguy force-pushed the geobl4 branch 5 times, most recently from a54599c to f75f645 Compare June 28, 2024 19:25
This raises ArgumentError because it was deprecated in 7.0 and
removed in 7.1.
# rubocop:disable Layout/LineLength
config.add_show_tools_partial :web_services, if: proc { |_context, _config, options|
options[:document] && (Settings.WEBSERVICES_SHOWN & options[:document].references.refs.map { |r| r.type.to_s }).any?
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this removed?

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like maybe the web services partial is now added by default in _show_sidebar.html.erb in GeoBlacklight so it no longer needs to be added a show tools partial separately. (Seems that way looking at GeoBlacklight itself)

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, we don't need to explicitly add this partial any longer

gem 'geo_monitor', '~> 0.7', github: 'geoblacklight/geo_monitor'
# Not compatible with GeoBlacklight 4.x
# https://github.com/geoblacklight/geo_monitor/issues/12
# gem 'geo_monitor', '~> 0.7', github: 'geoblacklight/geo_monitor'
Copy link
Contributor

Choose a reason for hiding this comment

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

What is our plan in general for GeoMonitor once this upgrade is in place? Will we need to work out a way later to use it, or will be table this for good? Thanks

Copy link
Contributor

Choose a reason for hiding this comment

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

I see this down below geoblacklight/geo_monitor#12 so I think that answers that question : )

@@ -3,6 +3,7 @@ class CheckLayerJob < ApplicationJob

##
# @param [GeoMonitor::Layer] layer
# TODO: migrate this to the geo_monitor gem itself?
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this still a to do? Thanks

Copy link
Member

Choose a reason for hiding this comment

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

It is; I added it as geoblacklight/geo_monitor#17 but left a reminder here for later. Something to do after GBLv4 compatibility.

@hudajkhan
Copy link
Contributor

Looks good in general to me. @dnoneill and I have a few questions. Since I've seen this branch functioning pretty well on staging already, happy to approve although I think we will need to hold off on merging because of indexing, etc.

@thatbudakguy thatbudakguy merged commit 52e1bd8 into main Jul 2, 2024
3 checks passed
@thatbudakguy thatbudakguy deleted the geobl4 branch July 2, 2024 15:45
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.

4 participants