Skip to content
This repository has been archived by the owner on May 22, 2024. It is now read-only.

Feature/fix multiple issues #390

Merged
merged 6 commits into from
Apr 13, 2018
Merged

Feature/fix multiple issues #390

merged 6 commits into from
Apr 13, 2018

Conversation

vrajmohan
Copy link
Contributor

No description provided.

Vraj Mohan added 4 commits April 11, 2018 14:32
This was inadvertently changed, which resulted in eregs on `dev` and
`stage` using the `prod` API
Due to psf/requests#4168, using https://user:password@url makes the
URL too long and results in a UnicodeError: "label empty or too long".

The workaround is to avoid specifying it in the URL and to use an
alternate mechanism of supplying credentials e.g. .netrc.
Call `eregs` directly as it is installed as an executable by
`regulations-parser`.

Remove call to migrate as it is only useful for the "local" case.
Copy link
Contributor

@ccostino ccostino left a comment

Choose a reason for hiding this comment

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

Thanks, @vrajmohan! I had a few questions as I went through but this looks like a straightforward fix overall!

general public to modify the regulatory data, so we need to authenticate.
Currently, this is implemented via HTTP Basic Auth and a very long user name
and password (effectively creating an API key). See the `HTTP_AUTH_USER` and
`HTTP_AUTH_PASSWORD` environment variables in cloud.gov for more.
Copy link
Contributor

Choose a reason for hiding this comment

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

However, there is a limit on how long each of these values can be, correct? Or has this been accounted for via another change/update?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know what the HTTP spec limit is for Basic Auth. By using ~/.netrc instead of specifying username:password@ in the URL, we are circumventing the psf/requests#4168 issue.

Copy link
Contributor

@ccostino ccostino Apr 12, 2018

Choose a reason for hiding this comment

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

That's what I was getting at, because that limit is definitely below the HTTP spec limit (which I was trying to look up the other day and not having much luck - it's somewhere in the RFC!). :-)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ha, I stand corrected! Per section 3.2.1 in the HTTP RFC:

...

The HTTP protocol does not place any a priori limit on the length of
a URI. Servers MUST be able to handle the URI of any resource they
serve, and SHOULD be able to handle URIs of unbounded length if they
provide GET-based forms that could generate such URIs. A server
SHOULD return 414 (Request-URI Too Long) status if a URI is longer
than the server can handle (see section 10.4.15).

Note: Servers ought to be cautious about depending on URI lengths
above 255 bytes, because some older client or proxy
implementations might not properly support these lengths.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TIL, user:password in URL is deprecated.
https://bugs.chromium.org/p/chromium/issues/detail?id=82250#c7

Copy link
Contributor

Choose a reason for hiding this comment

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

I also did not know IE hasn't supported this for some time according to those comments... TIL indeed!

```
machine fec-dev-eregs.app.cloud.gov # This should match the hostname from FEC_EREGS_API in the cf env output
login [copy and paste the value of HTTP_AUTH_USER from cf env output]
password [copy and paste the value of HTTP_AUTH_PASSWORD from cf env output]
Copy link
Contributor

Choose a reason for hiding this comment

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

These are the lines to edit in ~/.netrc? Sorry, was a bit confused at first, I thought they were supposed to be other environment variables!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, these are the lines to edit. After the edit, the file would look like:

machine fec-dev-eregs.app.cloud.gov 
login foobar
password foobars-top-secret-password

@@ -91,6 +102,8 @@ And monitor progress with
cf logs api | grep load-regs
```

Once this is successful, *delete the file `~/.netrc` from your local machine.*
Copy link
Contributor

Choose a reason for hiding this comment

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

Good call, thank you! :-)

@@ -51,11 +59,12 @@ In the environment you with to update regulations, first run:
$ cf env eregs
```
It will print to console the environment variables for the current running instance of eregs.
Use that console output to copy / paste the appropriate parameters here:
Use that console output to edit the file `~/.netrc` on your local machine.
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be nice if you add some description on why we need to save the credentials in .netrc folder before we load the eregs to our env's.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

@pkfec pkfec left a comment

Choose a reason for hiding this comment

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

Looks good!

@pkfec pkfec closed this Apr 13, 2018
@pkfec pkfec reopened this Apr 13, 2018
@pkfec pkfec merged commit 996aa37 into develop Apr 13, 2018
@ccostino ccostino deleted the feature/fix-multiple-issues branch April 13, 2018 19:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants