please review and publish the importio charm

Bug #1293635 reported by Ian clark
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Juju Charms Collection
Incomplete
Undecided
Ian clark

Bug Description

As above please review and pulish this charm to the juju store

Revision history for this message
Jorge Castro (jorge) wrote :

Hi Ian, you need to attach the branch to this bug, it's the (+ link a related branch) link on the right side of the page, thanks!

Revision history for this message
Ian clark (ian-clark) wrote :

Hi Jorge, I'd done that though maybe just after you checked, it should be there now (erm I can see it)

Revision history for this message
Charles Butler (lazypower) wrote :

Greetings Ian,

I'm pulling the charm now to start the review process and will have my notes momentarily. Thank you for this submission! I am excited to review the work after our charm school! GOOD LUCK!

Revision history for this message
Ian clark (ian-clark) wrote : Re: [Bug 1293635] Re: please review and publish the importio charm

Thanks Charles, looking forward to the feedback.

IC

On Tue, Mar 25, 2014 at 1:28 PM, Charles Butler <
<email address hidden>> wrote:

> Greetings Ian,
>
> I'm pulling the charm now to start the review process and will have my
> notes momentarily. Thank you for this submission! I am excited to review
> the work after our charm school! GOOD LUCK!
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1293635
>
> Title:
> please review and publish the importio charm
>
> Status in Juju Charms:
> New
>
> Bug description:
> As above please review and pulish this charm to the juju store
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/charms/+bug/1293635/+subscriptions
>

Revision history for this message
Charles Butler (lazypower) wrote :

Ian,

Thank you for the ImportIO charm submission, however I can't seem to figure out how to run a successful deployment. I've followed along in the readme, and read up on some of the linked documentation.

the config flags that stopped me in my tracks were: crawlTemplate and dataTemplate.

My thoughts to improve the UX here would be set a sane default crawl against a resource that can handle the load, so the charm is literally drag and deploy (with the minor setup of setting APIKey and UserGUID). Also extend the docs with an example deployment for a given environment.

Barring the feedback and updates to the readme, I look forward to the next iteration of the ImportIO charm. I'm going to set the status of this bug to incomplete. If you have any questions you can join us on #juju on irc.freenode.net, or contact us via the mailing list <email address hidden>.

When you are ready for another review please move the status of the bug to "Fix Committed" or "New" and someone will be along shortly to review your work.

Thanks again!

Changed in charms:
status: New → Incomplete
Revision history for this message
Ian clark (ian-clark) wrote :

Hi, have you seen my bundle: https://code.launchpad.net/~ian-clark/charms/bundles/importio-to-elasticsearch/bundle

sounds like I should include the defaults from there into this charm.

Revision history for this message
Charles Butler (lazypower) wrote :

Ian,

I had not seen that bundle. Thank you for pointing this out. I'll take
another look first thing in the morning and provide additional feedback.

Thanks again for your patience during the review process!

On Tue, Mar 25, 2014 at 12:35 PM, Ian clark <email address hidden> wrote:

> Hi, have you seen my bundle: https://code.launchpad.net/~ian-
> clark/charms/bundles/importio-to-elasticsearch/bundle
>
> sounds like I should include the defaults from there into this charm.
>
> --
> You received this bug notification because you are a member of charmers,
> which is subscribed to the bug report.
> https://bugs.launchpad.net/bugs/1293635
>
> Title:
> please review and publish the importio charm
>
> Status in Juju Charms:
> Incomplete
>
> Bug description:
> As above please review and pulish this charm to the juju store
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/charms/+bug/1293635/+subscriptions
>

Revision history for this message
Launchpad Janitor (janitor) wrote :

[Expired for Juju Charms Collection because there has been no activity for 60 days.]

Changed in charms:
status: Incomplete → Expired
Changed in charms:
status: Expired → Fix Committed
Revision history for this message
Matt Bruzek (mbruzek) wrote :

Hello Ian,

Thanks for your work on the import.io charm! The import.io crawler looks pretty cool and very useful. Looking forward to get import.io in the charm store because it will be another great relation for the elastic search charm. Very sorry for the delay but I had a chance to review the charm.

I was confused on how to properly configure the charm. Since these configuration values are not easy (or wise) to set defaults for, I would strongly suggest adding a more details on how to use the configuration options to the README file. The links to the import.io documentation are nice but some detail in the README would go a long way to make it easier for the user.

Looking at the configuration in the bundle helped me understand a little bit more, but I am still not an import.io expert at this point. The configuration is still confusing me.

I ran the 'charm proof' tool:
I: all charms should provide at least one thing

Informational messages from charm proof are OK, but I took a look. There is a comment in metadata.yaml that is not yet supported. So everything looks good here.

Install hook:

It is charm store policy that downloads must be cryptographically verified in charms. Otherwise the charm is vulnerable to unknown payloads being installed by the charms. You can see the policy details here: https://juju.ubuntu.com/docs/authors-charm-policy.html

What some charm authors have done is make the whole URL a configuration parameter and add a configuration parameter to have a sha1sum. Other approaches is to have the version configurable and have the known version sha1sums in the install hook to compare against. As the charm author this is up to you.

Overall this is a great charm and will be very beneficial to be in the charm store. I am going to put it in “incomplete” status for now. When you want another review put the bug back in “fix committed” status.

You can always reach out to me or any of the other charmers by email or in #juju on freenode.net if you have any other questions about the charm process. Thanks again for your work here.

Marco Ceppi (marcoceppi)
Changed in charms:
status: Fix Committed → Incomplete
assignee: nobody → Ian clark (ian-clark)
Revision history for this message
Ian clark (ian-clark) wrote :

Matt,

I will take a look at this, I have been out of the office for the past week.

IC

On Thu, Jul 24, 2014 at 7:25 PM, Matt Bruzek <email address hidden>
wrote:

> Hello Ian,
>
> Thanks for your work on the import.io charm! The import.io crawler
> looks pretty cool and very useful. Looking forward to get import.io in
> the charm store because it will be another great relation for the
> elastic search charm. Very sorry for the delay but I had a chance to
> review the charm.
>
> I was confused on how to properly configure the charm. Since these
> configuration values are not easy (or wise) to set defaults for, I would
> strongly suggest adding a more details on how to use the configuration
> options to the README file. The links to the import.io documentation
> are nice but some detail in the README would go a long way to make it
> easier for the user.
>
> Looking at the configuration in the bundle helped me understand a little
> bit more, but I am still not an import.io expert at this point. The
> configuration is still confusing me.
>
> I ran the 'charm proof' tool:
> I: all charms should provide at least one thing
>
> Informational messages from charm proof are OK, but I took a look.
> There is a comment in metadata.yaml that is not yet supported. So
> everything looks good here.
>
>
> Install hook:
>
> It is charm store policy that downloads must be cryptographically
> verified in charms. Otherwise the charm is vulnerable to unknown
> payloads being installed by the charms. You can see the policy details
> here: https://juju.ubuntu.com/docs/authors-charm-policy.html
>
> What some charm authors have done is make the whole URL a configuration
> parameter and add a configuration parameter to have a sha1sum. Other
> approaches is to have the version configurable and have the known
> version sha1sums in the install hook to compare against. As the charm
> author this is up to you.
>
> Overall this is a great charm and will be very beneficial to be in the
> charm store. I am going to put it in “incomplete” status for now. When
> you want another review put the bug back in “fix committed” status.
>
> You can always reach out to me or any of the other charmers by email or
> in #juju on freenode.net if you have any other questions about the charm
> process. Thanks again for your work here.
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1293635
>
> Title:
> please review and publish the importio charm
>
> Status in Juju Charms:
> Fix Committed
>
> Bug description:
> As above please review and pulish this charm to the juju store
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/charms/+bug/1293635/+subscriptions
>

Revision history for this message
Charles Butler (lazypower) wrote :

Greetings Ian, I see you sent an update but there hasn't been any activity in the repo.

Is this still something you're working on, or ran into a blocker that you cold use an extra pair of eyes to resolve?

Let us know if there's anything we can do to help.

All the best,

Charles

Revision history for this message
Ian clark (ian-clark) wrote :

Charles, Matt,

Sorry have been distracted by other issues recently. I will spend sometime on this week, to finish the submission, it seems I need to:-

1) cryptographically verified the crawler downloaded - this shouldn't be a problem

2) tackle the documentation issue - I can handle documentation, I had thought the bundle was the best place give detailed examples of a working setup? However can certainly add more documentation so it can exist in it's own right.

3) "all charms should provide at least one thing", currently has a commented out provides "http-json-stream" - not sure how to solve this problem, the charm once setup doesn't really provide anything except for maybe a stream of http json documents that get indexed in elasticsearch. Is this required? Is there an equivalent I could use?

Revision history for this message
José Antonio Rey (jose) wrote :

Thanks for replying, Ian.

Please, move the bug back to New or Fix Committed once you're ready for another review.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.