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

Adds a user agent to the metaphysics fetch #537

Merged
merged 1 commit into from
Feb 23, 2018
Merged

Conversation

orta
Copy link
Contributor

@orta orta commented Feb 20, 2018

This at least handles the reaction side of artsy/force#2191

Happy to change what the text actually is, but at least it's there.

@alloy
Copy link
Contributor

alloy commented Feb 20, 2018

Doesn’t matter much right now, but at some point Reaction will probably be used in other apps, so maybe the user-agent should also hint at what the host app is? I'm fine with merging now as-is, though.

@kanaabe
Copy link
Contributor

kanaabe commented Feb 20, 2018

It already is in Positron 😄

@joeyAghion
Copy link
Contributor

Yeah same thought... it would be nice if this was a default/fall-back and including systems could override it with their own agent identifiers. But this is much better than nothing!

@orta
Copy link
Contributor Author

orta commented Feb 21, 2018

If someone can show me an example of how we integrate in force/positron - I can see if we can have a config obj which can be include a user agent

@alloy
Copy link
Contributor

alloy commented Feb 21, 2018

@kanaabe My bad, thanks for rectifying! 🙏

@alloy
Copy link
Contributor

alloy commented Feb 21, 2018

One very explicit option, albeit somewhat repetitive, is to add a prop to the Artsy component for when no environment prop is specified

this.relayEnvironment = props.relayEnvironment || createEnvironment(this.currentUser)

<ContextProvider userAgent="Force" {...}>
  <SweetTree />
</ContextProvider>

@joeyAghion
Copy link
Contributor

Gonna merge this since it seems like an improvement on its own. Feel free to follow up with the configurability or not.

@joeyAghion joeyAghion merged commit 9b4e048 into master Feb 23, 2018
@orta orta deleted the relay_user_agent branch February 23, 2018 17:48
@orta
Copy link
Contributor Author

orta commented Feb 23, 2018

Agree, going to take a quick look at configurability now 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants