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

Imports in PHP and Python snippets don't respect the ApiVersion to import from correct package. #2050

Closed
SilasKenneth opened this issue May 15, 2024 · 4 comments · Fixed by #2047
Assignees
Labels
type: bug Something isn't working

Comments

@SilasKenneth
Copy link
Member

SilasKenneth commented May 15, 2024

Describe the bug
When generating the beta api snippets, this PHP snippet

use Microsoft\Graph\GraphServiceClient;
use Microsoft\Graph\Generated\Models\Event;
use Microsoft\Graph\Generated\Models\ItemBody;

should be

use Microsoft\Graph\Beta\GraphServiceClient;
use Microsoft\Graph\Beta\Generated\Models\Event;
use Microsoft\Graph\Beta\Generated\Models\ItemBody;

And this Python snippet

from msgraph import GraphServiceClient
from msgraph.generated.models.event import Event
from msgraph.generated.models.item_body import ItemBody

should be

from msgraph_beta import GraphServiceClient
from msgraph_beta.generated.models.event import Event
from msgraph_beta.generated.models.item_body import ItemBody

To Reproduce
Steps to reproduce the behavior:

  1. Go to '...'
  2. Click on '....'
  3. Scroll down to '....'
  4. See error

Expected behavior
A clear and concise description of what you expected to happen.

Screenshots
If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

  • OS: [e.g. iOS]
  • Browser [e.g. chrome, safari]
  • Version [e.g. 22]

Smartphone (please complete the following information):

  • Device: [e.g. iPhone6]
  • OS: [e.g. iOS8.1]
  • Browser [e.g. stock browser, safari]
  • Version [e.g. 22]

Additional context
Add any other context about the problem here.

@SilasKenneth SilasKenneth added the type: bug Something isn't working label May 15, 2024
@SilasKenneth SilasKenneth self-assigned this May 15, 2024
@SilasKenneth SilasKenneth linked a pull request May 15, 2024 that will close this issue
@shemogumbe
Copy link
Contributor

Concern on this, snippets are served to customer via GE and docs, if we include the APIVersion, will this be confusing for users.

  1. At generation stage, how do we check for API version, I believe we do not have this, and if we did, we are going to end up with two versions of the snippets, one for beta one for v1, the snippets should be the same except for the api e.g msgraph/msgraph_beta, is this a problem enought to get us to versions of snippets, nope.
  2. Does GE/docs have a way of falling back to V1 snippets where we have snippets reflecting V1 prefix/beta prefix for imports, if not, are we willing to build this.
  3. If we are not to have this reflect on Docs and GE, I suggest not having this in generation, instead, where we need this like in Raptor, we can add a simple if else check.
    Any thoughts on this @SilasKenneth

@sebastienlevert
Copy link

Based on the API version selected in GE or docs, we should match the same.

Both versions of the API should be able to live side by side in the same project. So my call would be to support the API version in the namespaces. We have it for other languages too...

@SilasKenneth
Copy link
Member Author

SilasKenneth commented May 16, 2024

Hi @shemogumbe

  1. For the first question, the method below does the check based on the URI that is provided by the user in the request to decide the metadata file to load.

    private (OpenApiSnippetMetadata, Uri) GetModelAndServiceUriTuple(Uri requestUri)

    Once that is decided, the loaded OpenAPI document will contain the version key.
    The Snippet model is created based on this information on
    var snippetModel = new SnippetModel(requestPayload, serviceRootUri.AbsoluteUri, openApiSnippetMetadata);

  2. For graph explorer, the version will always be passed to the request URL which always contains the API version as beta or v1.0, anything else will raise a 404 error on the graph explorer side and the devx api side.
    For the docs, there is Microsoft Graph REST API v1.0 endpoint reference - Microsoft Graph v1.0 | Microsoft Learn which is a reference doc for v1.0 hosting v1.0 snippets and Microsoft Graph REST API beta endpoint reference - Microsoft Graph beta | Microsoft Learn for beta which hosts the beta snippets.

  3. GE respects API version so does docs.


NOTE: I also answered to this in your comment to the PR.

@shemogumbe
Copy link
Contributor

Perftect, thanks for the clarification @SilasKenneth @sebastienlevert

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants