YangCatalog / module-compilation

Tools to collect, extract, and validate YANG modules
https://yangcatalog.org/private-page
Apache License 2.0
0 stars 5 forks source link

bbf-fast compilation status is unknown (but the tree view suggests that it's failed) #137

Closed wlupton closed 2 years ago

wlupton commented 2 years ago

This is probably related to the closed #112 and #119.

I uploaded some new modules yesterday and was checking https://yangcatalog.org/yang-search/module_details/bbf-fast. I see that the compilation status is unknown but if I click on the tree view to go to https://yangcatalog.org/api/services/tree/bbf-fast@2022-05-23.yang there's this message at the top of the file (no link to view the errors):

Info! This yang file contains some errors, but tree was created.

I looked at https://yangcatalog.org/private/BBF.json but the only entry for bbf-fast is for bbf-fast@2020-10-13.yang, i.e., it hasn't yet been updated with the latest revision (at what time of day does this happen?).

Also note:

wlupton commented 2 years ago

I noticed that iana-if-type@2022-03-07 doesn't appear to be in the YANG catalog. https://yangcatalog.org/yang-search/module_details/iana-if-type indicates that the latest version is @2021-06-21.

SlavomirMazurPantheon commented 2 years ago

Hello @wlupton,

thank you for reporting this issues.

  1. BBF modules should be populated now - unfortunately populating which was initialized by you was interrupted for some reason so I ran it once again. Mayne we should track running instances of populate script somehow.
  2. "Bootstrap Example" titles are fixed by this PR: https://github.com/YangCatalog/backend/pull/506 - the changes will take effect after the next build

Missing IANA modules: We are currently checking changes in iana-maintained modules and also automatically push new modules into YangModels/yang repository. However, it seems that we need some mechanism/cronjob which will populate these modules into YANG Catalog on regular basis. EDIT: IANA modules are in fact populated on daily basis, but there is some issue with populating iana-if-type@2022-03-07 - this is the reason why it is still missing in YANG Catalog

I will keep you informed in this thread.

Regards, Slavomir Mazur

wlupton commented 2 years ago

Thanks @SlavomirMazurPantheon. Just a few notes:

SlavomirMazurPantheon commented 2 years ago
wlupton commented 2 years ago

Is the elastic index search triggered by upload or is it a separate async process? If the former, perhaps the upload status could give the IDs of any triggered jobs? If the latter, is there an API to the elastic search?

I think it would be better to enable IANA check.sh so as to discover (and fix) problems early!

SlavomirMazurPantheon commented 2 years ago

Hello,

during modules population process, list of modules to index is dumped into file which is checked by cronjob - so this indexing phase runs separately, but is triggered by modules populating process.

I created check.sh script for "standard/iana" directory in this PR: https://github.com/YangModels/yang/pull/1250 After merging it should prevent merging corrupted yang files into "standard/iana" dir.

wlupton commented 2 years ago

Thanks.

wlupton commented 2 years ago

Does the new IANA check.sh script need to be added to the top-level testall.sh?

SlavomirMazurPantheon commented 2 years ago

Yes it should be added there, I forgot about this. I'm going to create a PR.

wlupton commented 2 years ago

I uploaded some more YANG the other day (job-id c8327acb-7df5-4c5e-89cd-c75fd25bb736). I'm not 100% sure it completed successfully (the job id is no longer there) but I see at least one of the new modules here (so I think it must have done):

https://yangcatalog.org/yang-search/module_details/bbf-grpc-client

However, this is showing a compilation status of "unknown"...

SlavomirMazurPantheon commented 2 years ago

Hello @wlupton,

yes, I saw that you added new modules and I was exactly careful if everything went as it should.

Also compilation should run once a day so then you should be able to see compilation status/result. However, we pull modules from YangModels/yang repository, and I can see your opened PR. Your modules will be compiled automatically only after they are merged into the main branch.

Regards, Slavomír Mazúr

wlupton commented 2 years ago

Ah OK. Yes it's true that the YangModels/yang PR is held up. But it seems inconsistent that the module should be pulled from YangModels/yang when its YANG catalog entry says to get it from the BBF repo directly (see below). Might there not be modules in the YANG catalog that will never be in YangModels/yang?

        "source-file": {
          "owner": "BroadbandForum",
          "path": "standard/common/bbf-grpc-client.yang",
          "repository": "yang"
        }
SlavomirMazurPantheon commented 2 years ago

These are 3 scripts that run separately and the first two of them (population to Redis and Elasticsearch) use this information provided in the payload so they pull modules from BroadbandForum/yang repo in this case.

The third one uses YangModels/yang, but if you are interested, it is not a problem to change it and use master branch of BroadbandForum/yang instead. Like it is really just a change on a few lines of code where we change the URL from where the BBF repo is to be cloned.

wlupton commented 2 years ago

Thanks. Up to you... but from my point of view it does seem better (indeed correct!) to use the source-file details from the catalog entry.

In looking into this I realised that our metadata doesn't include branch info:

    leaf branch {
      type string;
      description
        "Revision control system branch or tag to use to find the module.  If this is not
         specified, the head of the repository is used.";
    }

(BTW, Is head of the repository a well-defined term? Don't you have first to know which branch you're on. The branch should probably default to the default branch, which may or may not be master.)

We should specify branch in our metadata, and it should be a tag (probably a commit id would also work?) rather than a branch because past revisions won't be in HEAD. I'll try to ensure that we do this next time we upload YANG.

(Will it be OK to upload metadata that adds branch for existing revisions?)

SlavomirMazurPantheon commented 2 years ago

Hello,

The current implementation allows you to enter either the name of the branch (e.x.: master, main, dev ...) into the branch parameter, from which we will then obtain a commit hash or enter the commit hash directly. If you do not specify this property in your metadata, we will clone a repo to us and take the HEAD commit SHA -> so default branch is used and latest commit in this default branch. The branch property is used mainly to create schema property and also to retrieve the content of a file. If you are trying to add modules from latest commit in your repository no need to provide branch parameter.

wlupton commented 2 years ago

Thanks for the info.

Can I just confirm that the module update will fail if the file in HEAD doesn't have the requested yyyy-mm-dd revision as its latest revision?

I've looked at my code again, and realize that the uploaded metadata will only ever contain the latest revision of each module, so I don't think there's a problem.

SlavomirMazurPantheon commented 2 years ago

If your uploaded metadata contain the latest revision of each module each time there should not be any problem.

NOTE: PR in YangModels/yang was merged so you also should be able to see compilation results for individual modules that you uploaded.

wlupton commented 2 years ago

If your uploaded metadata contain the latest revision of each module each time there should not be any problem.

Right. It does, and there shouldn't be a problem. My question was a theoretical one :).

NOTE: PR in YangModels/yang was merged so you also should be able to see compilation results for individual modules that you uploaded.

Yes, I see them. Thanks. We still have iana-if-type@2021-06-21 :(.

SlavomirMazurPantheon commented 2 years ago

Yes - I'm still trying to find the best possible solution to solve this iana-if-type warning/error problem. However, if such a warning appears, it should not affect the final compilation-status, it should be PASSED.

wlupton commented 2 years ago

Thanks. Yes I noticed that the compilation had PASSED. I'll close this.