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

Process manager generate job ids #1209

Conversation

ricardogsilva
Copy link
Member

@ricardogsilva ricardogsilva commented Apr 7, 2023

Overview

This PR modifies the pygeoapi process manager to be responsible for generating job ids, as discussed in #1207.

The proposed implementation simply makes the generation of a job's id part of the process manager's set of responsabilities, which means that the job id is now returned from the process manager's execute_process() method, rather than it being an input to this method.

Related Issue / Discussion

Contributions and Licensing

(as per https://github.com/geopython/pygeoapi/blob/master/CONTRIBUTING.md#contributions-and-licensing)

  • I'd like to contribute [feature X|bugfix Y|docs|something else] to pygeoapi. I confirm that my contributions to pygeoapi will be compatible with the pygeoapi license guidelines at the time of contribution.
  • I have already previously agreed to the pygeoapi Contributions and Licensing Guidelines

@ricardogsilva ricardogsilva marked this pull request as ready for review April 11, 2023 10:42
@ricardogsilva
Copy link
Member Author

@tomkralidis

The failing test reported by CI seems to be unrelated to my changes - I've tracked it down to this failed request

https://demo.mapserver.org/cgi-bin/msautotest?version=1.3.0&service=WMS&request=GetMap&bbox=-90.0%2C-180.0%2C90.0%2C180.0&crs=EPSG%3A4326&layers=world_latlong&styles=default&width=500&height=300&format=image%2Fpng&transparent=TRUE

Indeed demo.mapserver.org is returning 502

@justb4
Copy link
Member

justb4 commented Apr 11, 2023

Yes MapServer demo is temporarily down due to upgrades, see also:
https://lists.osgeo.org/pipermail/mapserver-users/2023-April/083093.html

@@ -27,9 +27,10 @@
#
# =================================================================

from datetime import datetime
Copy link
Member

Choose a reason for hiding this comment

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

Can we keep the order of imports:

  • standard library (alphabetical order)
  • blank line
  • 3rd party imports (PyPI) (alphabetical order)
  • blank line
  • local imports (alphabetical order)

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought this was in line with keeping the order?

I usually put from imports below straight ones because it is also how isort and other tools do it, and because it is sorted alphabetically. Anyway, no biggie I'll switch it up 😉

@@ -28,6 +28,7 @@
# =================================================================

import logging
import uuid
Copy link
Member

Choose a reason for hiding this comment

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

Same comment here, RE: import ordering

@tomkralidis tomkralidis merged commit 5e9c82a into geopython:master Apr 12, 2023
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.

None yet

3 participants