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

Update getting-started.md #531

Merged
merged 3 commits into from
Jan 13, 2022
Merged

Update getting-started.md #531

merged 3 commits into from
Jan 13, 2022

Conversation

melsk-r
Copy link
Contributor

@melsk-r melsk-r commented Jan 5, 2022

Aanpassingen n.a.v. het verwijderen van de gegenereerde client code.

Aanpassingen n.a.v. het verwijderen van de gegenereerde client code.
2. [Bekijk de functionaliteit en specificaties](#functionaliteit)
3. [Probeer en test de API in de productie-omgeving](#probeer-en-test-de-api-in-de-productie-omgeving)

2. [Bekijk de functionaliteit en specificaties](#functionaliteit-en-specificaties)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wellicht handig om de paragraaf titels in de inhoudsopgave gelijk te houden met de titels van de paragrafen? Dus

  1. Bekijk de functionaliteit en specificaties
  2. [Implementeer API client](#implementeer-api-client]
    ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ik ben het deels met je eens.
Het betreft hier nl. niet zozeer een inhoudsopgave maar een stappenplan van waaruit wordt verwezen naar de paragrafen. Vandaar dat de tekst van de stappen mag afwijken van de paragraaftitel. De teksten achter de links moeten we echter wel gelijk maken aan de paragraaftitels.
Eigenlijk is dat ook wat je hierboven voorstelt en wat je denk ik ook bedoelt want je stelt bij 2. immers:

  1. Bekijk de functionaliteit en specificaties

voor en niet:

  1. functionaliteit en specificaties

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Zie mijn nieuwe voorstel.


2. [Bekijk de functionaliteit en specificaties](#functionaliteit-en-specificaties)
3. [Implementeer de API](#bouw-de-api)
4. [Probeer en test de API](#probeer-en-test-de-api)

## Aanmelden om aan te sluiten
[Vraag een API key voor de productieomgeving aan](https://formulieren.kadaster.nl/aanvraag_bag_api_huidige_bevragingen_productie){:target="_blank"}. Testen doe je bij voorkeur in de productie-omgeving.
Copy link
Contributor

Choose a reason for hiding this comment

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

Graag aanpassen in:' Testen doe je bij voorkeur in de testomgeving.'
Door testen in de productieomgeving worden ongecontroleerde aantallen requests ontvangen door nog niet gevonden fouten in de code die bijvoorbeeld loops veroorzaken. Hierdoor kan de beschikbaarheid van de API voor alle klanten onder druk komen te staan.
En idem als bij regel 8.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@NicoleKortoomsBAG ik herinner me dat we hebben afgesproken dat gebruikers in productie kunnen testen.

De acceptatieomgeving die ik gebruik voor nieuwe releases kunnen aansluiters niet gebruiken omdat die niet stabiel hoeft te zijn. Daarop komen niet volledig geteste nieuwe versies. Is er ook een testomgeving (eto) die altijd gelijk is aan productie waar aansluiters op kunnen testen? De aansluiting daarop (ten minste de url) moet dan ook worden toegevoegd aan de Getting started.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ongecontroleerde aantallen requests ontvangen door nog niet gevonden fouten in de code die bijvoorbeeld loops veroorzaken

Dat kan m.i. ook worden opgelost met rate limiting. Dat zouden we kunnen toevoegen als dat gewenst is. Zie https://geonovum.github.io/KP-APIs/API-strategie-extensies/#rate-limiting

Copy link
Contributor

@NicoleKortoomsBAG NicoleKortoomsBAG Jan 10, 2022

Choose a reason for hiding this comment

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

Er mag getest worden in productie, we kunnen ook niet zien of iets een testgeval is of een productievraag, maar de voorkeur is om in de acceptatieomgeving te testen. Als straks rate en quota limiting wordt toegepast, is het voor de afnemer nog belangrijker dat hij test in de testomgeving. Anders verbruikt hij zijn quota door testen.
Als de afnemer iets specifieks wil testen en de testomgeving dat geval niet kent, zal dat enkele geval geen probleem zijn voor zijn quota. Als hij nieuwe code test en er zit een loop in, kan hij zijn eigen productie stilleggen. Door de client het direct goed in te laten richten, voorkomen we dat en dat hij later extra werk moet doen.

Copy link
Collaborator

Choose a reason for hiding this comment

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

bij BRK hebben we naast productie acceptatieomgeving waar wij (Haal Centraal) nieuwe versies van de API op testen, en de ETO waar aansluiters op testen. De ETO is dus technisch altijd gelijk aan productie, maar bevat de data van de acceptatieomgeving. Dat is wat je voor BAG ook ook nodig hebt, wanneer je niet wilt dat op productie wordt getest.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ik heb hiervoor #533 aangemaakt

@@ -6,14 +6,14 @@ title: Getting Started

Om aan te sluiten kun je de volgende stappen doorlopen:
1. [Meld je aan bij het kadaster om toegang te krijgen tot de productieomgeving](#aanmelden-om-aan-te-sluiten)
Copy link
Contributor

Choose a reason for hiding this comment

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

bij het aanvragen van een API key, krijg je altijd een key voor de testomgeving (acceptatie) en de productieomgeving.

@@ -77,6 +77,19 @@ Verder zijn er nog een paar algemene functies die gelden voor alle bovenstaande
- Bij het zoeken van een pand op **locatie** moet de header **Content-Crs** worden meegestuurd. De API ondersteunt op dit moment alleen het RD coördinatenstelsel (epsg:28992).
Copy link
Collaborator

Choose a reason for hiding this comment

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

we hebben hier ook zoeken op bbox aan toegevoegd, zowel voor panden als voor adresseerbare objecten

Copy link
Collaborator

Choose a reason for hiding this comment

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

De content-crs header is niet meer verplicht. Wanneer je deze weglaat wordt default coördinatenstelsel RD (epsg:28992) gebruikt.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wijzigingen aangebracht n.a.v. je opmerkingen.

Copy link
Collaborator

@fsamwel fsamwel left a comment

Choose a reason for hiding this comment

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

de opmerkingen die @NicoleKortoomsBAG en ik hebben gemaakt staan los van het doel van deze PR. Deze kunnen dus ook in een aparte PR worden opgelost (maar moet wel worden opgelost).

Ik ben het wel eens met @MelvLee dat de paragraaftitels in de inhoudsopgave gelijk zouden moeten zijn aan de paragraaftitels waar ze naar verwijzen.


2. [Bekijk de functionaliteit en specificaties](#functionaliteit-en-specificaties)
3. [Implementeer de API](#bouw-de-api)
4. [Probeer en test de API](#probeer-en-test-de-api)

## Aanmelden om aan te sluiten
[Vraag een API key voor de productieomgeving aan](https://formulieren.kadaster.nl/aanvraag_bag_api_huidige_bevragingen_productie){:target="_blank"}. Testen doe je bij voorkeur in de productie-omgeving.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ik heb hiervoor #533 aangemaakt

@@ -77,6 +77,19 @@ Verder zijn er nog een paar algemene functies die gelden voor alle bovenstaande
- Bij het zoeken van een pand op **locatie** moet de header **Content-Crs** worden meegestuurd. De API ondersteunt op dit moment alleen het RD coördinatenstelsel (epsg:28992).
- [HAL links](https://tools.ietf.org/html/draft-kelly-json-hal-08){:target="_blank"}, die soms [templated](https://github.com/VNG-Realisatie/Haal-Centraal-common/blob/v1.1.0/features/uri-templating.feature){:target="_blank"} worden geleverd.

## Bouw de API
Client code kan worden gegenereerd vanuit de "[genereervariant](https://github.com/VNG-Realisatie/Haal-Centraal-BAG-bevragen/blob/master/specificatie/genereervariant/openapi.yaml){:target="_blank" rel="noopener"}" van de API-specificaties en een code generator. Een overzicht van code generatoren is te vinden op [OpenAPI.Tools](https://openapi.tools/#sdk){:target="_blank" rel="noopener"}.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Client code kun je genereren met de .....
Een overzicht met codegeneratoren kun je vinden op....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aangepast.

## Bouw de API
Client code kan worden gegenereerd vanuit de "[genereervariant](https://github.com/VNG-Realisatie/Haal-Centraal-BAG-bevragen/blob/master/specificatie/genereervariant/openapi.yaml){:target="_blank" rel="noopener"}" van de API-specificaties en een code generator. Een overzicht van code generatoren is te vinden op [OpenAPI.Tools](https://openapi.tools/#sdk){:target="_blank" rel="noopener"}.

Deze repo bevat scripts waarmee met behulp van [OpenAPI Generator](https://openapi-generator.tech/){:target="_blank" rel="noopener"} client code kan worden gegenereerd in JAVA, .NET (Full Framework & Core) en Python. De makkelijkste manier om de code generatie scripts te gebruiken, is door deze repo te clonen. Na het clonen kan met `npm install` de benodigde packages worden geïnstalleerd. Vervolgens kan met `npm run <script naam>` één van de volgende scripts worden uitgevoerd:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Deze repo bevat scripts waarmee je met.....client code kunt genereren in JAVA, .NET (Full Framework & Core) en Python.

Na het clonen kun je met npm install de benodigde packages installeren en kun je met npm run <script naam> één van de volgende scripts uitvoeren:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aangepast.

- oas:generate-python-client (voor Python client code)

Een lijst met andere ondersteunde generator opties is te vinden in de [Generators List](https://openapi-generator.tech/docs/generators){:target="_blank" rel="noopener"} van OpenAPI Generator.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Een lijst met andere ondersteunde generator opties kun je vinden...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aangepast.

Een lijst met andere ondersteunde generator opties is te vinden in de [Generators List](https://openapi-generator.tech/docs/generators){:target="_blank" rel="noopener"} van OpenAPI Generator.

Note. De prerequisite OpenAPI Generator is JAVA. Een JAVA runtime moet worden geïnstalleerd voordat OpenAPI Generator kan worden gebruikt.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Je moet een JAVA runtime installeren voordat je OpenAPI Generator kunt gebruiken.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aangepast.

Copy link
Collaborator

Choose a reason for hiding this comment

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

'De prerequisite OpenAPI Generator is JAVA' aanvullen naar 'De prerequisite van OpenAPI Generator is JAVA'?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aangepast

Aanpassingen n.a.v. opmerkingen van Frank en Cathy.
@@ -74,9 +74,23 @@ Verder zijn er nog een paar algemene functies die gelden voor alle bovenstaande
- Gebruik van paginering om het aantal zoekresultaten per zoekvraag te beperken. Met de **page** parameter kan een volgende pagina worden gevraagd. Met de **pageSize** parameter kan gekozen worden voor meer of minder zoekresultaten per pagina (standaard is 20, maximum is 100). Voor werking, zie feature [paginering](https://github.com/VNG-Realisatie/Haal-Centraal-common/blob/v1.1.0/features/paginering.feature){:target="_blank"}
- Soms kan er een onderzoek lopen of een gegeven wel correct is. Er zijn dan twijfels over de juistheid van de geregistreerde waarde. De API levert deze waarde wel, maar neemt die velden dan op in **mogelijkOnjuist** met de waarde True.
- Sommige resources bevatten geometrie. De API ondersteunt op dit moment alleen het RD coördinatenstelsel (epsg:28992). Bij een aanvraag die geometrie teruglevert moet de request header **Accept-Crs** worden meegestuurd.
- Bij het zoeken van een pand op **locatie** moet de header **Content-Crs** worden meegestuurd. De API ondersteunt op dit moment alleen het RD coördinatenstelsel (epsg:28992).
- Bij het zoeken van een pand op **locatie** mag de header **Content-Crs** worden meegestuurd. Wanneer je deze header weglaat wordt default coördinatenstelsel RD (epsg:28992) gebruikt wat op dit moment ook het enige het coördinatenstelsel is dat de API ondersteunt.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Frank had al eerder aangegeven dat dit ook geldt voor het zoeken met de bbox. Die aanvulling is nog niet gedaan.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Die aanvulling staat in regel 78.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Overheen gelezen. Excuus.

@melsk-r melsk-r requested a review from JohanBoer January 13, 2022 09:47
@@ -74,9 +74,23 @@ Verder zijn er nog een paar algemene functies die gelden voor alle bovenstaande
- Gebruik van paginering om het aantal zoekresultaten per zoekvraag te beperken. Met de **page** parameter kan een volgende pagina worden gevraagd. Met de **pageSize** parameter kan gekozen worden voor meer of minder zoekresultaten per pagina (standaard is 20, maximum is 100). Voor werking, zie feature [paginering](https://github.com/VNG-Realisatie/Haal-Centraal-common/blob/v1.1.0/features/paginering.feature){:target="_blank"}
- Soms kan er een onderzoek lopen of een gegeven wel correct is. Er zijn dan twijfels over de juistheid van de geregistreerde waarde. De API levert deze waarde wel, maar neemt die velden dan op in **mogelijkOnjuist** met de waarde True.
- Sommige resources bevatten geometrie. De API ondersteunt op dit moment alleen het RD coördinatenstelsel (epsg:28992). Bij een aanvraag die geometrie teruglevert moet de request header **Accept-Crs** worden meegestuurd.
- Bij het zoeken van een pand op **locatie** moet de header **Content-Crs** worden meegestuurd. De API ondersteunt op dit moment alleen het RD coördinatenstelsel (epsg:28992).
- Bij het zoeken van een pand op **locatie** mag de header **Content-Crs** worden meegestuurd. Wanneer je deze header weglaat wordt default coördinatenstelsel RD (epsg:28992) gebruikt wat op dit moment ook het enige het coördinatenstelsel is dat de API ondersteunt.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Overheen gelezen. Excuus.

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.

6 participants