Remove deprecated multi-project methods in Enterprise GeoIP downloader #130062
Conversation
samxbr
added
>non-issue
:Data Management/Ingest Node
elasticsearchmachine
added
the
Team:Data Management
|
Pinging @elastic/es-data-management (Team:Data Management) |
samxbr
changed the title
| void updateDatabases() throws IOException { | ||
| var clusterState = clusterService.state(); | ||
| var geoipIndex = clusterState.getMetadata().getProject().getIndicesLookup().get(EnterpriseGeoIpDownloader.DATABASES_INDEX); | ||
| var geoipIndex = clusterState.getMetadata() |
There was a problem hiding this comment.
Could we maybe assign a local variable like this (i.e. move the annotation)
@NotMultiProjectCapable(description = "Enterprise GeoIP not available in serverless")
final var projectId = ProjectId.DEFAULT;That makes it easier to understand/read why we're hardcoding the default project ID here as there is only one reference to the default project ID and it has the annotation directly above.
There was a problem hiding this comment.
Similar to EnterpriseGeoIpDownloaderTaskExecutor, there will likely be more changes needed than just the project ID (project ID is the easy one to spot). I can make a local variable on the project ID, I think it's better to also keep the class level annotation as a reminder to review the whole class again.
| import static org.elasticsearch.ingest.geoip.GeoIpDownloaderTaskExecutor.ENABLED_SETTING; | ||
| import static org.elasticsearch.ingest.geoip.GeoIpDownloaderTaskExecutor.POLL_INTERVAL_SETTING; | ||
|
|
||
| @NotMultiProjectCapable(description = "Enterprise GeoIP not available in serverless") |
There was a problem hiding this comment.
I don't immediately see why we need this annotation here, am I missing something? Can you move it to the specific lines that need it?
There was a problem hiding this comment.
There are probably many lines that need this if we put it at line level, and some are unclear to me how it will work yet (for example the secure settings). The intention for this annotation at class level is that the whole class needs to be reviewed again on which specific lines needs to be changed when it become serverless available.
There was a problem hiding this comment.
I can update the description to make the intention clearer
3 participants
This doesn't make Enterprise GeoIP downloader project-aware, it simply removes the deprecated methods. Since there are quite some work involved in making these classes project-aware, and Enterprise GeoIP is not available in serverless anyway, we will mark these classes
@NotMultiProjectCapableand revisit them later when Enterprise GeoIP becomes available in serverless.