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

ArcGIS Server Mapservice probe added #454

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

LuukS
Copy link

@LuukS LuukS commented Jul 18, 2023

Now it is possible to probe ArcGIS Server MapServices

Now it is possible to probe ArcGIS Server MapServices
@justb4 justb4 added this to the Version 0.9.1 milestone Jul 18, 2023
Copy link
Member

@justb4 justb4 left a comment

Choose a reason for hiding this comment

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

Thanks, but before merging this needs to be addressed/added:

  • major: esrims.py is mostly identical to the existing esrifs.py. Apart from naming I see only two differences in the Probe execution: "OBJECTID"(MS) vs 'objectIdFieldName' (FS) and if layer['subLayerIds'] is None: (MS). For maintenance it is better to merge all common code into a common baseclass like ESRIDrilldown and specialize subclasses ESRIMSDrilldown and ESRIFSDrilldown. I am not familiar with ESRI MS (though I wrote the FS Probe) but would expect to get actual raster maps, test for image content etc? So for example rename esrifs.py to esri.py and have all code (Basclass and two subclasses) in a single file. For example at some point the Authentication may need to be improved.
  • add tests: basically only need to add to tests/data/fixtures.json (look for ESRI:FS how that is done).
  • add documentation in docs/plugins.rst (look for ESRI:FS )
  • add ESRI:MS to check in GeoHealthCheck/models.py line 433
  • add ESRI:MS to check in GeoHealthCheck/healthcheck.py line 246

Maybe some other places, best is to search for 'ESRI' or 'ESRI:FS' in source tree.

Thanks for patience!

…ed esri

The complete URL of the Feature- of MapService is the input for the probe. Based on the last part of the url (FeatureServer or MapServer) the code uses another json field to get the identityfield.
A drilldown test is niet possible in case of a query test because the url contains the number of the layer to test. Due to this the drilldown test gives an error.
@LuukS
Copy link
Author

LuukS commented Aug 17, 2023

I have changed the code and created an new way to test Feature- or MapServices based on the complete url of a service. I added this change in my repository. Can you have a look at it if it is OK now?

@PedroVenancio
Copy link

Hi @LuukS

I'm seeing that you are assuming in esrims.py the ID field always as OBJECTID:
https://github.com/geopython/GeoHealthCheck/pull/454/files#diff-b4de69f543dade9bc51e38f054d90e0977b562a66a33569447d737cefe198c9aR159

This is not always the case and GeoHealthCheck.plugins.probe.esrims.ESRIMSDrilldown fails when OBJECTID field is not present in that layer.

Is it possible to check the layer fields, search for esriFieldTypeOID field type and get obj_id_field_name from there?

Thanks!

@justb4
Copy link
Member

justb4 commented Aug 17, 2023

I quickly glanced through the changes, but am a bit confused: I see the resource type changed from ESRI:FS to ESRI.
This will affect existing installations. My idea (see above) is to keep ESRI:FS and add ESRI:MS. But that the implementation uses a common baseclass. But I still see the full file esrims.py. Nowhere do I see the class GeoHealthCheck.plugins.probe.esri.ESRIDrilldown while it is refered in config_main.yml.

@LuukS
Copy link
Author

LuukS commented Aug 17, 2023 via email

@justb4
Copy link
Member

justb4 commented Apr 9, 2024

Any way you/we can progress this @LuukS ?

@LuukS
Copy link
Author

LuukS commented Aug 13, 2024

Hi @LuukS

I'm seeing that you are assuming in esrims.py the ID field always as OBJECTID: https://github.com/geopython/GeoHealthCheck/pull/454/files#diff-b4de69f543dade9bc51e38f054d90e0977b562a66a33569447d737cefe198c9aR159

This is not always the case and GeoHealthCheck.plugins.probe.esrims.ESRIMSDrilldown fails when OBJECTID field is not present in that layer.

Is it possible to check the layer fields, search for esriFieldTypeOID field type and get obj_id_field_name from there?

Thanks!

I changed to code in the latest commit to do this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants