Warn if cog.yaml or predictor are missing

[asingh9530]

What issue this PR fix - #1452

[zeke]

Thanks for the PR. It's a little confusing that this warning describes two possible issues. What do you think about handling the two exceptions with different warnings?

Some untested pseudocode:

except ConfigDoesNotExist:
        warnings.warn("no cog.yaml found")
except PredictorNotSet:
        warnings.warn("no predict() method found in Predictor")

[asingh9530]

@zeke yeah I think that is much better let me make those changes.

[zeke]

So... this will print a warning, but it will also blow up because the pass was removed, right?

Is that that the intended behavior here, or do we want to print a warning without failing?

[asingh9530]

So... this will print a warning, but it will also blow up because the pass was removed, right?

I think even in previous setting it would blow but without any warning or error but in this case it will show warning to user and then blow. So the user will likely know the root cause.

Is that that the intended behavior here, or do we want to print a warning without failing?

if we want to prevent the build to even begin then we have to raise exception instead of warning something like this.

    except ConfigDoesNotExist:
        raise ConfigDoesNotExist("no cog.yaml found or present")
    except PredictorNotSet:
        raise PredictorNotSet("no predict method found in Predictor")

what do you think ?

[zeke]

This looks good to me 👍🏼

[asingh9530]

@zeke @aron can you please approve test workflows.

[asingh9530]

@zeke could please check why Integration failed as this PR dosen't contain any changes to the file reflected in tests.

[yorickvP]

@asingh9530 the test 'https://github.com/replicate/cog/blob/main/test-integration/test_integration/test_build.py#L9' fails because it tests for a specific error message, which has been changed in this PR. Please update the test to match the new error message.

[mattt]

Thanks for your help with this, @asingh9530!

[zeke]

Thanks for picking this up @mattt! 🙏🏼